Skip to content

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Jul 31, 2025

Fixes #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:

let parent_is_const =
self.outer_trait_or_trait_impl.as_ref().and_then(TraitOrTraitImpl::constness).is_some();
match &item.kind {
AssocItemKind::Fn(func)
if parent_is_const
|| ctxt == AssocCtxt::Trait
|| matches!(func.sig.header.constness, Const::Yes(_)) =>
{
self.visit_attrs_vis_ident(&item.attrs, &item.vis, &func.ident);
let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), &item.vis, &*func);
self.visit_fn(kind, item.span, item.id);
}
AssocItemKind::Type(_) => {
let disallowed = (!parent_is_const).then(|| match self.outer_trait_or_trait_impl {

Instead, I believe it's sufficient to simply "reset" [const] allowness whenever we enter a new block.

@rustbot
Copy link
Collaborator

rustbot commented Jul 31, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jul 31, 2025
@@ -1,7 +1,3 @@
//@ compile-flags: -Znext-solver
Copy link
Member Author

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?

@rust-log-analyzer

This comment has been minimized.

@ShoyuVanilla ShoyuVanilla force-pushed the tilde-const-in-block branch from 5ec2c20 to c02624b Compare July 31, 2025 16:29
@@ -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
Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Jul 31, 2025

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

Copy link
Member

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.

Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Jul 31, 2025

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!

@ShoyuVanilla ShoyuVanilla marked this pull request as draft July 31, 2025 17:13
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2025
@fmease fmease added the PG-const-traits Project group: Const traits label Jul 31, 2025
@ShoyuVanilla ShoyuVanilla force-pushed the tilde-const-in-block branch from c02624b to 7d78968 Compare August 3, 2025 16:49
@ShoyuVanilla ShoyuVanilla marked this pull request as ready for review August 3, 2025 16:49
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 3, 2025
@ShoyuVanilla
Copy link
Member Author

So, I’ve prohibited the following:

  • Any usage of [const] Trait bounds on ADTs, regardless of position—including inside const fn bodies.
  • Any local usages of [const] Trait bounds (I doubt there would be any case other than impl [const] Trait within impl_trait_in_binding) inside anonymous constants, such as in const generic arguments. They are still allowed in legal places like const fn bodies.

@fee1-dead
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 4, 2025

📌 Commit 7d78968 has been approved by fee1-dead

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 Aug 4, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 4, 2025
… 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.
bors added a commit that referenced this pull request Aug 4, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PG-const-traits Project group: Const traits 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.

~const is allowed on structs (and other bad positions) within const fn.
7 participants