-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use ErrorGuaranteed::unchecked_claim_error_was_emitted
less
#104554
Conversation
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! This is very nice, just had some nitpicking (and a potentially scary pre-existing incremental unsoundness?).
@@ -1218,9 +1218,13 @@ impl<'tcx> InferCtxt<'tcx> { | |||
); | |||
|
|||
if self.tcx.sess.err_count() > self.err_count_on_creation { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh dear, this is causally flawed... nowadays we can have multiple InferCtxt
s alive at the same time (for different query instances on the stack, that depend on eachother).
The delay_span_bug
should never cause an issue here but... err_count_on_creation
could cause incremental unsoundness due to errors produced elsewhere in the system.
At the very least this kind of thing needs far more clever logic to save/restore counts/deltas when entering/exiting InferCtxt
s and I'm not even sure it's worth it... (cc @jackh726 @nikomatsakis).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thought we don't cache anything on disc if there are any errors during this compilation session? 🤔
r? @eddyb 😸 |
0b9224f
to
5db72a4
Compare
@@ -1,4 +1,3 @@ | |||
// compile-flags:-Ztreat-err-as-bug=5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I uh ???? :(
714fb1e
to
df3625f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits, then r=me
@bors r+ rollup |
📌 Commit 72d1c77025b949548207dd264b6b95e3ae6ed071 has been approved by It is now in the queue for this repository. |
@bors r- waiting for ci |
72d1c77
to
d80a9a2
Compare
@bors r=lcnr |
📌 Commit d80a9a2f887fcdc50d46589ce10a128e8ef1c8db has been approved by It is now in the queue for this repository. |
b6ba1e0
to
c712ed1
Compare
c712ed1
to
45a09a4
Compare
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#101162 (Migrate rustc_resolve to use SessionDiagnostic, part # 1) - rust-lang#103386 (Don't allow `CoerceUnsized` into `dyn*` (except for trait upcasting)) - rust-lang#103405 (Detect incorrect chaining of if and if let conditions and recover) - rust-lang#103594 (Fix non-associativity of `Instant` math on `aarch64-apple-darwin` targets) - rust-lang#104006 (Add variant_name function to `LangItem`) - rust-lang#104494 (Migrate GUI test to use functions) - rust-lang#104516 (rustdoc: clean up sidebar width CSS) - rust-lang#104550 (fix a typo) Failed merges: - rust-lang#104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less) r? `@ghost` `@rustbot` modify labels: rollup
@bors r=lcnr |
Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less there are only like 3 or 4 call sites left after this but it wasnt obvious to me how to remove them
Rollup of 8 pull requests Successful merges: - rust-lang#104001 (Improve generating Custom entry function) - rust-lang#104411 (nll: correctly deal with bivariance) - rust-lang#104528 (Properly link `{Once,Lazy}{Cell,Lock}` in docs) - rust-lang#104553 (Improve accuracy of asinh and acosh) - rust-lang#104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less) - rust-lang#104566 (couple of clippy::perf fixes) - rust-lang#104575 (deduplicate tests) - rust-lang#104580 (diagnostics: only show one suggestion for method -> assoc fn) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#101162 (Migrate rustc_resolve to use SessionDiagnostic, part # 1) - rust-lang#103386 (Don't allow `CoerceUnsized` into `dyn*` (except for trait upcasting)) - rust-lang#103405 (Detect incorrect chaining of if and if let conditions and recover) - rust-lang#103594 (Fix non-associativity of `Instant` math on `aarch64-apple-darwin` targets) - rust-lang#104006 (Add variant_name function to `LangItem`) - rust-lang#104494 (Migrate GUI test to use functions) - rust-lang#104516 (rustdoc: clean up sidebar width CSS) - rust-lang#104550 (fix a typo) Failed merges: - rust-lang#104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less) r? `@ghost` `@rustbot` modify labels: rollup
there are only like 3 or 4 call sites left after this but it wasnt obvious to me how to remove them