-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
merge need_type_info_err(_const)
#77093
Conversation
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've just skimmed the implementation for now: I'll take a closer look later. But I'm really happy to see this code cleaned up!
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
--> $DIR/issue-77092.rs:13:26 | ||
| | ||
LL | println!("{:?}", take_array_from_mut(&mut arr, i)); | ||
| ^^^^^^^^^^^^^^^^^^^ cannot infer the value of the constant `{_: usize}` |
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.
Would it make more sense here to use a similar message to below?
E.g.
cannot infer the value of const parameter `N`
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.
the failed inference variable has a ConstInference
origin, as it was created from unifying two uninferred consts vids.
So we don't have the info to emit a better error message in this case. We probably should "just" use one of the ConstParam
s as the origin in that case. I think the reason we don't do so with types is due to variance.
Not completely sure about all of this though, so I would prefer to look into this in a followup PR
Implement the name changes and put the result of previously I am myself not completely happy with the new names but wasn't able to think of anything better 😆 |
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.
Actual implementation looks good, so r=me after renaming ty
and if you're happy with the names.
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
@bors r=varkor rollup |
📌 Commit 9a607c0 has been approved by |
…r=varkor merge `need_type_info_err(_const)` I hoped that this would automatically solve rust-lang#76737 but it doesn't quite seem like it fixes rust-lang#77092 r? @varkor
Rollup of 12 pull requests Successful merges: - rust-lang#75454 (Explicitly document the size guarantees that Option makes.) - rust-lang#76631 (Add `x.py setup`) - rust-lang#77076 (Add missing code examples on slice iter types) - rust-lang#77093 (merge `need_type_info_err(_const)`) - rust-lang#77122 (Add `#![feature(const_fn_floating_point_arithmetic)]`) - rust-lang#77127 (Update mdBook) - rust-lang#77161 (Remove TrustedLen requirement from BuilderMethods::switch) - rust-lang#77166 (update Miri) - rust-lang#77181 (Add doc alias for pointer primitive) - rust-lang#77204 (Remove stray word from `ClosureKind::extends` docs) - rust-lang#77207 (Rename `whence` to `span`) - rust-lang#77211 (Remove unused #[allow(...)] statements from compiler/) Failed merges: - rust-lang#77170 (Remove `#[rustc_allow_const_fn_ptr]` and add `#![feature(const_fn_fn_ptr_basics)]`) r? `@ghost`
I hoped that this would automatically solve #76737 but it doesn't quite seem like it
fixes #77092
r? @varkor