Fix where-bound suggestion with legacy const generics#158981
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @khyperia (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
thanks for looking into this! as noted by @asquared31415 in the issue, generic_const_exprs is on its way out, to be replaced by generic_const_args (min_generic_const_args to start with). It would be nice if we could get a repro with min_generic_const_args instead if at all possible, but it's fine to keep this testing generic_const_exprs as well I suppose. Some feedback:
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dd697af to
9ea03db
Compare
|
Thank you for the review. I forgot to compile the suggestions and blindly trusted them. I updated the fix to wrap parentheses around non-atomic expressions with the help of About
|
|
apologies, I've fallen ill and probably won't get around to reviewing this for a while, so I'm rerolling r? compiler |
|
@rustbot author |
9ea03db to
5798f26
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
5798f26 to
77a7d21
Compare
|
I appreciate your suggestions, I've implemented them all. One more thing, since the suggestion is @rustbot ready |
That would be nice :) I will approve this after adding @rustbot author |
Point the suggestion at the const argument's span instead of the whole obligation, and parenthesize the snippet when the const expression needs it (e.g. `(N + 1) as usize`) so the suggested bound parses and compiles. Also add regression tests for legacy const generic where-bound suggestions.
77a7d21 to
4882893
Compare
|
@rustbot ready |
…-bound-suggestion, r=mu001999
Fix where-bound suggestion with legacy const generics
## Summary
During lowering of legacy const generics in `lower_legacy_const_generics`, the function's span was passed when creating the associated `LocalDefId` instead of the const argument span. This later caused diagnostics to pull the function's name instead of the const argument for suggestions when emitting an `unconstrained generic constant` error.
Also adds regression tests for all basic integer types. The full set isn't redundant: non-`usize` types exercise the `as usize` cast in the suggestion, while `usize` exercises the no-cast path. The tests use a cross-crate aux crate rather than a target-specific intrinsic (the rewrite only fires cross-crate), so they run on all platforms.
### Sample
```rust
#![allow(incomplete_features)]
#![feature(generic_const_exprs)]
use std::arch::x86_64::*;
fn uwu<const N: i32>() {
unsafe {
let a = _mm_setzero_ps();
let b = _mm_setzero_ps();
_mm_shuffle_ps(a, b, N + 1);
}
}
```
### Previous Output
```rust
error: unconstrained generic constant
--> src/lib.rs:9:30
|
9 | _mm_shuffle_ps(a, b, N + 1);
| ^^^^^
|
help: try adding a `where` bound
|
5 | fn uwu<const N: i32>() where [(); _mm_shuffle_ps as usize]: {
| ++++++++++++++++++++++++++++++++++++
```
### Current Output
```rust
error: unconstrained generic constant
--> src/lib.rs:9:30
|
9 | _mm_shuffle_ps(a, b, N + 1);
| ^^^^^
|
help: try adding a `where` bound
|
5 | fn uwu<const N: i32>() where [(); N + 1 as usize]: {
| +++++++++++++++++++++++++++
```
## AI Assistance Disclosure
This is my first time contributing to the compiler, and I used Claude Opus 4.8 to assist with finding the root cause and implementing the fix, and to navigate the contributing docs, build system, and debugging tooling. The regression tests were designed and written entirely by me. I used Grok to proofread the written materials (commit bodies and this PR) for spelling and grammar. I reviewed and verified all changes myself.
## Related Issues
Closes rust-lang#108382
Rollup of 6 pull requests Successful merges: - #159039 (resolve: fix effective visibilities for items in ambiguous glob sets) - #156047 (Fix trait method resolution on an adjusted never type) - #158981 (Fix where-bound suggestion with legacy const generics) - #159061 (Skip trivial cast lint for trait object upcasts) - #159248 (compiler: Remove `-Zmutable-noalias`) - #159250 (Rename `errors.rs` file to `diagnostics.rs` (13/N))
Rollup of 6 pull requests Successful merges: - #159039 (resolve: fix effective visibilities for items in ambiguous glob sets) - #158981 (Fix where-bound suggestion with legacy const generics) - #159061 (Skip trivial cast lint for trait object upcasts) - #159241 (Update books) - #159248 (compiler: Remove `-Zmutable-noalias`) - #159250 (Rename `errors.rs` file to `diagnostics.rs` (13/N))
Rollup merge of #158981 - coroama-contrib:fix-const-generics-bound-suggestion, r=mu001999 Fix where-bound suggestion with legacy const generics ## Summary During lowering of legacy const generics in `lower_legacy_const_generics`, the function's span was passed when creating the associated `LocalDefId` instead of the const argument span. This later caused diagnostics to pull the function's name instead of the const argument for suggestions when emitting an `unconstrained generic constant` error. Also adds regression tests for all basic integer types. The full set isn't redundant: non-`usize` types exercise the `as usize` cast in the suggestion, while `usize` exercises the no-cast path. The tests use a cross-crate aux crate rather than a target-specific intrinsic (the rewrite only fires cross-crate), so they run on all platforms. ### Sample ```rust #![allow(incomplete_features)] #![feature(generic_const_exprs)] use std::arch::x86_64::*; fn uwu<const N: i32>() { unsafe { let a = _mm_setzero_ps(); let b = _mm_setzero_ps(); _mm_shuffle_ps(a, b, N + 1); } } ``` ### Previous Output ```rust error: unconstrained generic constant --> src/lib.rs:9:30 | 9 | _mm_shuffle_ps(a, b, N + 1); | ^^^^^ | help: try adding a `where` bound | 5 | fn uwu<const N: i32>() where [(); _mm_shuffle_ps as usize]: { | ++++++++++++++++++++++++++++++++++++ ``` ### Current Output ```rust error: unconstrained generic constant --> src/lib.rs:9:30 | 9 | _mm_shuffle_ps(a, b, N + 1); | ^^^^^ | help: try adding a `where` bound | 5 | fn uwu<const N: i32>() where [(); N + 1 as usize]: { | +++++++++++++++++++++++++++ ``` ## AI Assistance Disclosure This is my first time contributing to the compiler, and I used Claude Opus 4.8 to assist with finding the root cause and implementing the fix, and to navigate the contributing docs, build system, and debugging tooling. The regression tests were designed and written entirely by me. I used Grok to proofread the written materials (commit bodies and this PR) for spelling and grammar. I reviewed and verified all changes myself. ## Related Issues Closes #108382
Summary
During lowering of legacy const generics in
lower_legacy_const_generics, the function's span was passed when creating the associatedLocalDefIdinstead of the const argument span. This later caused diagnostics to pull the function's name instead of the const argument for suggestions when emitting anunconstrained generic constanterror.Also adds regression tests for all basic integer types. The full set isn't redundant: non-
usizetypes exercise theas usizecast in the suggestion, whileusizeexercises the no-cast path. The tests use a cross-crate aux crate rather than a target-specific intrinsic (the rewrite only fires cross-crate), so they run on all platforms.Sample
Previous Output
Current Output
AI Assistance Disclosure
This is my first time contributing to the compiler, and I used Claude Opus 4.8 to assist with finding the root cause and implementing the fix, and to navigate the contributing docs, build system, and debugging tooling. The regression tests were designed and written entirely by me. I used Grok to proofread the written materials (commit bodies and this PR) for spelling and grammar. I reviewed and verified all changes myself.
Related Issues
Closes #108382