Skip to content

Conversation

@Bryntet
Copy link

@Bryntet Bryntet commented Dec 14, 2025

Small PR that ports the #[rustc_legacy_const_generics] to use the new attribute parser!

r? JonathanBrouwer

@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2025

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 Dec 14, 2025
@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from 2c36833 to 9e9e9a4 Compare December 14, 2025 19:24
@rustbot

This comment has been minimized.

@jdonszelmann
Copy link
Contributor

@rusbot author

@Bryntet
Copy link
Author

Bryntet commented Dec 14, 2025

@rusbot author

@jdonszelmann I think you pinged the wrong account earlier

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from 2f0fddb to 5daf590 Compare December 14, 2025 21:40
@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
Copy link
Contributor

Make sure to squash these commits later

@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch 2 times, most recently from 9a71d29 to 108cdd1 Compare December 14, 2025 22:51
@jdonszelmann
Copy link
Contributor

r? jdonszelmann
(@JonathanBrouwer feel free to co review, but I already started a bit)

@Bryntet
Copy link
Author

Bryntet commented Dec 15, 2025

I'm having trouble getting ahold of the attributes using the parse_limited like you mentioned

I have the following code that compiles, but it doesn't actually actually work (tests fail because of missing const function arguments)

let Res::Def(DefKind::Fn, def_id) = self.partial_res_map.get(&expr.id)?.full_res()? else {
    return None;
};

// We only support cross-crate argument rewriting. Uses
// within the same crate should be updated to use the new
// const generics style.
if def_id.is_local() {
    return None;
}
if !expr.attrs.is_empty() {
    dbg!(&expr.attrs);
}
let Some(hir::Attribute::Parsed(AttributeKind::RustcLegacyConstGenerics { fn_indexes, .. })) = AttributeParser::parse_limited(tcx.sess, &expr.attrs, sym::rustc_legacy_const_generics, expr.span, expr.id, Some(tcx.features())) else {
    return None;
};

Some(fn_indexes.iter().map(|(num, _)| *num).collect())

I think it's that the expr.attrs is empty, so I tried passing in attrs from compiler/rustc_ast_lowering/src/expr.rs:L105 but that seems to have the same issue

I did a read of the CI results and it does seem like the performance issues are big.

@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 Dec 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@JonathanBrouwer JonathanBrouwer self-assigned this Dec 15, 2025
@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from 108cdd1 to 2d245a4 Compare December 15, 2025 08:28
@rust-log-analyzer

This comment has been minimized.

@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from 511bed0 to 98a10a8 Compare December 15, 2025 10:22
@Bryntet

This comment was marked as resolved.

@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 Dec 15, 2025
@Bryntet
Copy link
Author

Bryntet commented Dec 15, 2025

@rustbot author

@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 Dec 15, 2025
@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from 98a10a8 to ee55202 Compare December 15, 2025 10:31
@Bryntet
Copy link
Author

Bryntet commented Dec 15, 2025

@rustbot ready

@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 Dec 15, 2025
Copy link
Contributor

@JonathanBrouwer JonathanBrouwer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me,jana after Jana is also happy and possibly after fixing my last nitpick

View changes since this review

@Bryntet Bryntet force-pushed the brynte/parse_legacy_const_generic_args branch from ee55202 to f550532 Compare December 15, 2025 11:00
@jdonszelmann
Copy link
Contributor

@bors r=jonathanbrouwer,jdonszelmann

@bors
Copy link
Collaborator

bors commented Dec 15, 2025

📌 Commit f550532 has been approved by jonathanbrouwer,jdonszelmann

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 Dec 15, 2025
@jdonszelmann
Copy link
Contributor

@bors rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2025
…generic_args, r=jonathanbrouwer,jdonszelmann

Port `#[rustc_legacy_const_generics]` to use attribute parser

Small PR that ports the `#[rustc_legacy_const_generics]` to use the new attribute parser!

r? JonathanBrouwer
bors added a commit that referenced this pull request Dec 15, 2025
Rollup of 9 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149354 (Bootstrap config: libgccjit libs dir)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser )
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2025
…generic_args, r=jonathanbrouwer,jdonszelmann

Port `#[rustc_legacy_const_generics]` to use attribute parser

Small PR that ports the `#[rustc_legacy_const_generics]` to use the new attribute parser!

r? JonathanBrouwer
bors added a commit that referenced this pull request Dec 15, 2025
Rollup of 8 pull requests

Successful merges:

 - #148756 (Warn on codegen attributes on required trait methods)
 - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf)
 - #149271 (feat: dlopen Enzyme)
 - #149459 (std: sys: fs: uefi: Implement set_times and set_perm)
 - #149950 (Simplify how inline asm handles `MaybeUninit`)
 - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser )
 - #150014 (Metadata loader cleanups)
 - #150021 (document that mpmc channels deliver an item to (at most) one receiver)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar
Copy link
Member

Looks like this failed in rollup due to docs relying on a removed import: #150030 (comment)

@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 Dec 16, 2025
@bors
Copy link
Collaborator

bors commented Dec 16, 2025

☔ The latest upstream changes (presumably #143924) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

7 participants