Skip to content

InferCtxt tainted_by_errors_flag should be Option<ErrorGuaranteed>#100368

Merged
bors merged 2 commits intorust-lang:masterfrom
chenyukang:fix-100321
Aug 23, 2022
Merged

InferCtxt tainted_by_errors_flag should be Option<ErrorGuaranteed>#100368
bors merged 2 commits intorust-lang:masterfrom
chenyukang:fix-100321

Conversation

@chenyukang
Copy link
Member

@chenyukang chenyukang commented Aug 10, 2022 •
edited by compiler-errors
Loading

Fixes #100321.
Use Cell<Option> to guarantee that we emit an error when that flag is set.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 10, 2022
@rust-highfive
Copy link
Contributor

rust-highfive commented Aug 10, 2022

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2022
lcnr
lcnr reviewed Aug 10, 2022
View reviewed changes
@lcnr
Copy link
Contributor

lcnr commented Aug 10, 2022

r? @lcnr

@rust-highfive rust-highfive assigned lcnr and unassigned wesleywiser Aug 10, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr
Copy link
Contributor

lcnr commented Aug 10, 2022

ok, that's worrying 😅 you should be able to get a backtrace of where we incorrectly set tainted_by_errors by using RUSTFLAGS=-Ztreat-err-as-bug when compiling the stage 1 library. I don't actually know the best way to do that 😁

alternatively, change the delay_span_bug call to a bug!() and check where compiling std fails.

can look into this myself in case you get stuck or overwhelmed 😁 sorry, this is not an E-easy issue anymore

@chenyukang
Copy link
Member Author

chenyukang commented Aug 11, 2022

ok, that's worrying 😅 you should be able to get a backtrace of where we incorrectly set tainted_by_errors by using RUSTFLAGS=-Ztreat-err-as-bug when compiling the stage 1 library. I don't actually know the best way to do that 😁

alternatively, change the delay_span_bug call to a bug!() and check where compiling std fails.

can look into this myself in case you get stuck or overwhelmed 😁 sorry, this is not an E-easy issue anymore

Maybe we found another bug.
Will take some time to dig deeper tonight, I still need to learn a lot on debugging compiler.

@lcnr
Copy link
Contributor

lcnr commented Aug 11, 2022

yeah, I am pretty sure that the reason CI is failing is a bug of the current compiler, your PR looks correct.

@chenyukang
Copy link
Member Author

chenyukang commented Aug 20, 2022

yeah, I am pretty sure that the reason CI is failing is a bug of the current compiler, your PR looks correct.

seems caused by this line of code,

rust/compiler/rustc_trait_selection/src/traits/mod.rs

Line 467 in dd01122

infcx.set_tainted_by_errors();

I commented this line, the crash disappear 🤣

per my understanding, we should only call set_tainted_by_errors when there are errors, but how we fix this?

@lcnr
Copy link
Contributor

lcnr commented Aug 20, 2022

we probably do get some unwanted overflow errors if we remove that, don't we? 🤔 if so fixing that might be difficult.

I guess that requires a more general change to the way fulfillment deals with overflow... would have to take some time to experiment with this locally in that case.

@chenyukang
Copy link
Member Author

chenyukang commented Aug 22, 2022

we probably do get some unwanted overflow errors if we remove that, don't we? 🤔 if so fixing that might be difficult.

I guess that requires a more general change to the way fulfillment deals with overflow... would have to take some time to experiment with this locally in that case.

Comfirmed with @cjgillot, this hack used to prob overflow ICE, I tested in local and seems we don't get any overflow diagnostic any more, so I will remove this line.

@lcnr
Copy link
Contributor

lcnr commented Aug 23, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 23, 2022

📌 Commit f466a75 has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 23, 2022
@Dylan-DPC
Rollup merge of rust-lang#100368 - chenyukang:fix-100321, r=lcnr
41e8b97 
InferCtxt tainted_by_errors_flag should be Option<ErrorGuaranteed>

Fixes rust-lang#100321.
Use Cell<Option<ErrorGuaranteed>> to guarantee that we emit an error when that flag is set.
This was referenced Aug 23, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2022
@bors
Auto merge of rust-lang#100920 - Dylan-DPC:rollup-vlcw3sr, r=Dylan-DPC
a1bea15 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#99249 (Do not re-parse function signatures to suggest generics)
 - rust-lang#100309 (Extend comma suggestion to cases where fields arent missing)
 - rust-lang#100368 (InferCtxt tainted_by_errors_flag should be Option<ErrorGuaranteed>)
 - rust-lang#100768 (Migrate `rustc_plugin_impl` to `SessionDiagnostic`)
 - rust-lang#100835 (net listen backlog update, follow-up from rust-lang#97963.)
 - rust-lang#100851 (Fix rustc_parse_format precision & width spans)
 - rust-lang#100857 (Refactor query modifier parsing)
 - rust-lang#100907 (Fix typo in UnreachableProp)
 - rust-lang#100909 (Minor `ast::LitKind` improvements)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f42cdf7 into rust-lang:master Aug 23, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcnr lcnr lcnr left review comments

Assignees

@lcnr lcnr

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.65.0

Development

Successfully merging this pull request may close these issues.

InferCtxt::tainted_by_errors_flag should be Option<ErrorGuaranteed>

7 participants

@chenyukang @rust-highfive @lcnr @rust-log-analyzer @bors @wesleywiser @rustbot

Comments