-
Notifications
You must be signed in to change notification settings - Fork 13.6k
fix: Error on illegal [const]
s inside blocks within legal positions
#144741
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
base: master
Are you sure you want to change the base?
Conversation
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
@@ -1,7 +1,3 @@ | |||
//@ compile-flags: -Znext-solver |
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 guess this flags were introduced to prevent triggering other ICEs but we don't need this anymore?
This comment has been minimized.
This comment has been minimized.
5ec2c20
to
c02624b
Compare
tests/ui/unpretty/exhaustive.rs
Outdated
@@ -807,7 +807,6 @@ mod types { | |||
let _: impl Send + 'static; //[hir]~ ERROR `impl Trait` is not allowed | |||
let _: impl 'static + Send; //[hir]~ ERROR `impl Trait` is not allowed | |||
let _: impl ?Sized; //[hir]~ ERROR `impl Trait` is not allowed | |||
let _: impl [const] Clone; //[hir]~ ERROR `impl Trait` is not allowed |
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.
This one used to pass ast passes - and failing tests with this PR - but I think it shouldn't
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.
Hm, yeah this should definitely still be allowed. impl [const] Clone
is valid in a const fn with the impl_trait_in_bindings
, and it should for example work in this case:
const fn foo<T: [const] Foo>(x: &T) {
let y: &impl [const] Foo = x;
}
impl_trait_in_bindings
is probably just broken with [const]
-ness. If you could fix the AST validation, then that would be great. Otherwise, I'd at least like to understand why it is broken so it can have a proper FIXME in the codebase and so we can file an issue to fix it as a follow-up.
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.
Oh, I wasn't aware of impl_trait_in_bindings
😅 I'll try fixing this AST validation with proper interaction with that feature. Thanks!
c02624b
to
7d78968
Compare
So, I’ve prohibited the following:
|
@bors r+ rollup |
… r=fee1-dead fix: Error on illegal `[const]`s inside blocks within legal positions Fixes rust-lang#132067 I initially considered moving `[const]` validations to `rustc_ast_lowering`, but that approach would require adding constness information to `AssocCtxt`, which introduces significant changes - especially within `rustc_expand` - just to support a single use case here: https://github.com/rust-lang/rust/blob/3fb1b53a9dbfcdf37a4b67d35cde373316829930/compiler/rustc_ast_passes/src/ast_validation.rs#L1596-L1610 Instead, I believe it's sufficient to simply "reset" `[const]` allowness whenever we enter a new block.
Rollup of 5 pull requests Successful merges: - #144741 (fix: Error on illegal `[const]`s inside blocks within legal positions) - #144779 (Implement debugging output of the bootstrap Step graph into a DOT file) - #144813 (Add a tidy check to prevent adding UI tests directly under `tests/ui/`) - #144866 (Remove `SHOULD_EMIT_LINTS` in favor of `should_emit`) - #144887 (`rust-analyzer` subtree update) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #132067
I initially considered moving
[const]
validations torustc_ast_lowering
, but that approach would require adding constness information toAssocCtxt
, which introduces significant changes - especially withinrustc_expand
- just to support a single use case here:rust/compiler/rustc_ast_passes/src/ast_validation.rs
Lines 1596 to 1610 in 3fb1b53
Instead, I believe it's sufficient to simply "reset"
[const]
allowness whenever we enter a new block.