Skip to content
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

Disentangle ForwardGenericParamBan and ConstParamTy ribs #138259

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

compiler-errors
Copy link
Member

In #137617, the ConstParamTy rib was adjusted to act kinda like the ForwardGenericParamBan. However, this means that it no longer served its purpose banning generics from parent items. Although we still are checking for param type validity using the ConstParamTy_ trait, which means that we weren't accepting code we shouldn't, I think it's a bit strange for us not to be rejecting code like this during resolution and instead letting these malformed const generics leak into the type system:

trait Foo<T> {
  fn bar<const N: T>() {}
}

This PR does a few things:

  1. Introduce a ForwardGenericParamBanReason enum, and start using the ForwardGenericParamBan rib to ban forward-declared params in const tys when generic_const_parameter_types is enabled.
  2. Start using the ConstParamTy rib to ban all generics when generic_const_parameter_types is disabled.
  3. Improve the diagnostics for both of the cases above, and for forward-declared params in parameter defaults too :3

r? @BoxyUwU or reassign

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 9, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 9, 2025

ah right 🤔 I guess at the time I thought that params would include all params in scope but that doesnt make any sense xd

@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 9, 2025

#![feature(adt_const_params)]

trait Trait<const N: usize> {
    fn foo<const M: [u8; N]>() {}
}

Can you add a test that we forbid this? Apparently we don't have such a test already ^^'

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Mar 9, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 11, 2025

@bors r+

this is much nicer now thx

@bors
Copy link
Contributor

bors commented Mar 11, 2025

📌 Commit a79f97b has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
…r=BoxyUwU

Disentangle `ForwardGenericParamBan` and `ConstParamTy` ribs

In rust-lang#137617, the `ConstParamTy` rib was adjusted to act kinda like the `ForwardGenericParamBan`. However, this means that it no longer served its purpose banning generics from *parent items*. Although we still are checking for param type validity using the `ConstParamTy_` trait, which means that we weren't accepting code we shouldn't, I think it's a bit strange for us not to be rejecting code like this during *resolution* and instead letting these malformed const generics leak into the type system:

```rust
trait Foo<T> {
  fn bar<const N: T>() {}
}
```

This PR does a few things:
1. Introduce a `ForwardGenericParamBanReason` enum, and start using the `ForwardGenericParamBan` rib to ban forward-declared params in const tys when `generic_const_parameter_types` is enabled.
2. Start using the `ConstParamTy` rib to ban *all* generics when `generic_const_parameter_types` is disabled.
3. Improve the diagnostics for both of the cases above, and for forward-declared params in parameter defaults too :3

r? `@BoxyUwU` or reassign
@jieyouxu
Copy link
Member

jieyouxu commented Mar 12, 2025

I think this failed in rollup: #138382 (comment)

2025-03-12T04:34:54.7238690Z ---- [ui] tests/ui/const-generics/generic_const_parameter_types/references-parent-generics.rs#nofeat stdout ----
2025-03-12T04:34:54.7239912Z Saved the actual stderr to "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-generics/generic_const_parameter_types/references-parent-generics.nofeat/references-parent-generics.nofeat.stderr"
2025-03-12T04:34:54.7240808Z diff of stderr:
2025-03-12T04:34:54.7240928Z 
2025-03-12T04:34:54.7241108Z -	error: `Self` is forbidden as the type of a const generic parameter
2025-03-12T04:34:54.7241628Z +	error[E0770]: the type of const parameters must not depend on other generic parameters
2025-03-12T04:34:54.7242657Z 2	  --> $DIR/references-parent-generics.rs:7:25
2025-03-12T04:34:54.7242960Z 3	   |
2025-03-12T04:34:54.7243405Z 4	LL |     type Assoc<const N: Self>;
2025-03-12T04:34:54.7243593Z 
2025-03-12T04:34:54.7243683Z -	   |                         ^^^^
2025-03-12T04:34:54.7243923Z -	   |
2025-03-12T04:34:54.7244201Z -	   = note: the only supported types are integers, `bool`, and `char`
2025-03-12T04:34:54.7244646Z +	   |                         ^^^^ the type must not depend on the parameter `Self`

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 12, 2025
@jieyouxu jieyouxu closed this Mar 12, 2025
@jieyouxu jieyouxu reopened this Mar 12, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 12, 2025

gamer

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

@bors r=BoxyUwU

@bors
Copy link
Contributor

bors commented Mar 12, 2025

📌 Commit 42773bf has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 12, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
…earth

Rollup of 12 pull requests

Successful merges:

 - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`)
 - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.)
 - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj)
 - rust-lang#138259 (Disentangle `ForwardGenericParamBan` and `ConstParamTy` ribs)
 - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`)
 - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js)
 - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more)
 - rust-lang#138357 (merge `TypeChecker` and `TypeVerifier`)
 - rust-lang#138394 (remove unnecessary variant)
 - rust-lang#138403 (Delegation: one more ICE fix for `MethodCall` generation)
 - rust-lang#138407 (Delegation: reject C-variadics)
 - rust-lang#138409 (Use sa_sigaction instead of sa_union.__su_sigaction for AIX)

r? `@ghost`
`@rustbot` modify labels: rollup
@compiler-errors
Copy link
Member Author

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2025
…earth

Rollup of 12 pull requests

Successful merges:

 - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`)
 - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.)
 - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj)
 - rust-lang#138259 (Disentangle `ForwardGenericParamBan` and `ConstParamTy` ribs)
 - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`)
 - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js)
 - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more)
 - rust-lang#138357 (merge `TypeChecker` and `TypeVerifier`)
 - rust-lang#138394 (remove unnecessary variant)
 - rust-lang#138403 (Delegation: one more ICE fix for `MethodCall` generation)
 - rust-lang#138407 (Delegation: reject C-variadics)
 - rust-lang#138409 (Use sa_sigaction instead of sa_union.__su_sigaction for AIX)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2d7a592 into rust-lang:master Mar 13, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants