-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Allow self in const generics #157949
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
Open
bb1yd
wants to merge
1
commit into
rust-lang:main
Choose a base branch
from
bb1yd:allow-self-in-const-generics
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+48
−3
Open
Allow self in const generics #157949
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Allow Self in const generics when Self doesn't depends on generics | ||
| trait MyTrait { | ||
| fn foo<const N: i32>(); | ||
| } | ||
|
|
||
| impl MyTrait for i32 { | ||
| fn foo<const N: Self>() {} | ||
| } | ||
| impl<T> Wrap<T> { | ||
| fn f<const N: Self>() {} | ||
| //~^ ERROR the type of const parameters must not depend on other generic parameters | ||
|
|
||
| } | ||
|
|
||
| struct Wrap<T>(T); | ||
| fn main(){} |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| error[E0770]: the type of const parameters must not depend on other generic parameters | ||
| --> $DIR/allow-self-in-const-generics.rs:10:19 | ||
| | | ||
| LL | fn f<const N: Self>() {} | ||
| | ^^^^ the type `Wrap<T>` must not depend on other generic parameter | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0770`. |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
IINM you're now accepting this code despite
Ndepending onTwhich shouldn't be allowed w/o enabling GCPT.View changes since the review
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.
Is there a way to check if
Selfdepends on generics?Uh oh!
There was an error while loading. Please reload this page.
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'm not super well-versed in name resolution, so I don't know if there's a good way to do this in
rustc_resolvethat still nicely leverages the existing rib structure. Boxy probably has an idea.Calling
type_ofhere on theLocalDefIdfound in theSelfTyAliasis probably too prone to cycles or hangs; looking athir_nodecorresp. to theLocalDefIdmight be an option but that might not integrate with the visitor.Naively speaking, I would do it in HIR ty lowering by inspecting the type as returned by
type_ofand by reusing+generalizingcheck_assoc_const_binding_typebut that's probably also not super ideal. I haven't read a lot of the new mGCA code in HIR ty lowering, maybe there's already a function for that (that's similar tocheck_assoc_const_binding_type?)?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.
Is it okay to check if
Selfhas param incheck_param_wf? I saw that we do other checks on const params here, but I think it doesn't belong to the HIR ty lowering process.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.
we used to have a nice way of handling this and no longer do 😅 previously you could set a flag on the
Selfto tell hir ty lowering to not support generic parameters on it 🤔one problem with checking this in HIR ty lowering is we'd not error on this code (probably) due to the fact that the lowered ty wouldn't have the type alias
Foo<N>it'd just haveBar... 🤔i think that's probably just fine... such code will be accepted long term once
generic_const_parameter_typesis stabilized anyway 🤷♀️ though- can you feature gate support forSelfundermin_adt_const_params/adt_const_params, I don't want to immediately stabilize this but it is something we should just support when stabilizing type aliases here (which we are doing with adt const params)I would either do this, or do it in
fn type_ofwhen lowering the type of const parameters. I think I prefertype_ofas you can then returnTyKind::Errorfor the type of the const parameter which should avoid the rest of the compiler needing to handle types of const parameters having generics with GCPT enabled