-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't report numeric inference ambiguity when we have previous errors #95751
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
@@ -79,35 +79,7 @@ LL | trait bar { fn dup(&self) -> Self; fn blah<X>(&self); } | |||
= note: required because of the requirements on the impl of `CoerceUnsized<Box<dyn bar>>` for `Box<{integer}>` | |||
= note: required by cast to type `Box<dyn bar>` | |||
|
|||
error[E0283]: type annotations needed |
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.
hehe, wonderful to see this issue getting better too
Not sure this is the best fix. It would be better, I think to fallback even under error. |
@jackh726, like in general? that would have a bunch of unintended side-effects i think -- the choice to not fallback when infcx is tainted by errors is intentional apparently: https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_typeck/check/fallback.rs.html#92 |
If we're falling back to |
So when we |
I'll investigate modifying the fallback logic to unconditionally apply float and integer fallback even when infcx is tainted, but i'm assuming that'll have additional diagnostic fallout that may need discussion on its merit, or extra patching-up to suppress, so i will keep that separate from this PR. |
&& crate_names.len() == 1 | ||
&& ["`core`", "`alloc`", "`std`"].contains(&crate_names[0].as_str()) | ||
&& spans.len() == 0 | ||
&& (crate_names.len() == 1 |
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.
Could this check just be simplified to "if the self type is a numeric infer"?
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.
So we need to check for int/float variables not just in self, but in the trait's substs as well, so we don't report the things that implement ConcreteType: Trait<{integer}>
.
Also removing the core/alloc/std crate name check leads to some regressions in errors reported with simd stuff and when we're unable to infer const variables due to other errors :/
@bors r+ rollup |
📌 Commit 39bff4b has been approved by |
Don't report numeric inference ambiguity when we have previous errors Fixes rust-lang#95648
Don't report numeric inference ambiguity when we have previous errors Fixes rust-lang#95648
Don't report numeric inference ambiguity when we have previous errors Fixes rust-lang#95648
Don't report numeric inference ambiguity when we have previous errors Fixes rust-lang#95648
Don't report numeric inference ambiguity when we have previous errors Fixes rust-lang#95648
Rollup of 8 pull requests Successful merges: - rust-lang#90066 (Add new ThinBox type for 1 stack pointer wide heap allocated trait objects) - rust-lang#95374 (assert_uninit_valid: ensure we detect at least arrays of uninhabited types) - rust-lang#95599 (Strict provenance lints) - rust-lang#95751 (Don't report numeric inference ambiguity when we have previous errors) - rust-lang#95764 ([macro_metavar_expr] Add tests to ensure the feature requirement) - rust-lang#95787 (reword panic vs result section to remove recoverable vs unrecoverable framing) - rust-lang#95797 (Remove explicit delimiter token trees from `Delimited`.) - rust-lang#95804 (rustdoc: Fix empty doc comment with backline ICE) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #95648