Visit param_env field in Obligation's TypeFoldable impl
#91205
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I'm not sure either 🙃 |
camelid
reviewed
Nov 25, 2021
camelid
added
the
T-compiler
|
the output probably changed cause the param_env of the obligation also contained an erroneous anon const which we previously did not compute for whatever reason 🤷 slightly prefer to keep the old formatting here, apart from that r=me r? @lcnr |
Aaron1011
force-pushed
the
visit_param_env
branch
from
06d62b6
to
d7e8212
Compare
|
@bors r=lcnr |
|
📌 Commit d7e8212 has been approved by |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 25, 2021
Visit `param_env` field in Obligation's `TypeFoldable` impl This oversight appears to have gone unnoticed for a long time without causing issues, but it should still be fixed.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 25, 2021
Visit `param_env` field in Obligation's `TypeFoldable` impl This oversight appears to have gone unnoticed for a long time without causing issues, but it should still be fixed.
bors
added
S-waiting-on-author
This oversight appears to have gone unnoticed for a long time without causing issues, but it should still be fixed.
Aaron1011
force-pushed
the
visit_param_env
branch
from
d7e8212
to
a7cc6bc
Compare
|
I've blessed the Clippy test. @bors r=lcnr |
|
📌 Commit a7cc6bc has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Aaron1011
added a commit
to Aaron1011/rust
that referenced
this pull request
Nov 26, 2021
Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
|
Finished benchmarking commit (1e79d79): comparison url. Summary: This change led to very large relevant regressions 😿 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
|
Can someone file a ticket to follow up with changing the output here? Inference errors outside of expression context are very confusing to people. |
|
The perf regression is really unfortunate - I had hoped that #91254 would allow us to recover some of the loss, but it appears to have no impact. I'll see if there are any call sites that don't actually care about the |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 27, 2021
…lcnr Only check for errors in predicate when skipping impl assembly Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
Aaron1011
added a commit
to Aaron1011/rust
that referenced
this pull request
Nov 27, 2021
Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 28, 2021
…lcnr Only check for errors in predicate when skipping impl assembly Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Dec 2, 2021
Visit `param_env` field in Obligation's `TypeFoldable` impl This oversight appears to have gone unnoticed for a long time without causing issues, but it should still be fixed.
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Dec 2, 2021
Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
pietroalbini
pushed a commit
to pietroalbini/rust
that referenced
this pull request
Jan 19, 2022
Prior to PR rust-lang#91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR rust-lang#91205 are reverted.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This oversight appears to have gone unnoticed for a long time
without causing issues, but it should still be fixed.