InferCtxt tainted_by_errors_flag should be Option<ErrorGuaranteed>

[chenyukang]

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

[rust-highfive]

r? @wesleywiser

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

[lcnr]

r? @lcnr

[lcnr]

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]

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]

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

[chenyukang]

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]

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]

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]

@bors r+ rollup

[bors]

📌 Commit f466a75 has been approved by lcnr

It is now in the queue for this repository.