Port `#[rustc_as_ptr]` to the new attribute system #142498

GrigorenkoPV · 2025-06-14T10:36:26Z

It might make sense to introduce some new parser analogous to Single, but even more simple: for parsing attributes that take no arguments and may appear only once (such as #[rustc_as_ptr] or #[rustc_const_stable_indirect]). Not sure if this should be a single impl parsing all such attributes, or one impl per attribute. Or how it will play along with the upcoming rework of attribute validation. Or how these argumentless attributes should be called (I've loosely referred to them as markers in the name of the new module in this PR, but not sure how good it is).

This is a part of #131229, so
r? @jdonszelmann

For reference, the #[rustc_as_ptr] attribute was created back in #132732 as a followup to #128985.

rustbot · 2025-06-14T10:36:33Z

Some changes occurred in compiler/rustc_attr_data_structures

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

mejrs

Or how these argumentless attributes should be called (I've loosely referred to them as markers in the name of the new module in this PR, but not sure how good it is).

"markers" already means quite a lot, see https://doc.rust-lang.org/nightly/nightly-rustc/rustc_attr_parsing/index.html for example.

Anyway, your questions seem to be about code organization - I think it would be better to organize attributes based on what they do rather than how they are parsed.

a single impl parsing all such attributes

The downside of this approach is that updating an attribute to take an argument would get annoying. Say if we had just #[blah] and then changed it to #[blah(alot|maybe| <nothing>)] then that would require moving all the code related to parsing blah, rather than just making changes in it.

compiler/rustc_attr_parsing/src/attributes/markers.rs

compiler/rustc_attr_parsing/src/attributes/mod.rs

jdonszelmann · 2025-06-14T14:25:21Z

compiler/rustc_passes/src/check_attr.rs

@@ -147,6 +147,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                    | AttributeKind::ConstStabilityIndirect
                    | AttributeKind::MacroTransparency(_),
                ) => { /* do nothing  */ }
+                Attribute::Parsed(AttributeKind::AsPtr) => {
+                    self.check_applied_to_fn_or_method(hir_id, attr, span, target)


this is an ICE, check_applied_to_fn_or_method runs attr.span() which aren't supported on parsed attributes

no it isn't, look

rust/compiler/rustc_passes/src/check_attr.rs

Line 1835 in 86039cc

attr_span: attr.span(),

It still calls attr.span()

no it isn't, look

rust/compiler/rustc_passes/src/check_attr.rs

Line 1835 in 86039cc

attr_span: attr.span(),

It still calls attr.span()

Which works fine, because of
https://github.com/rust-lang/rust/pull/142498/files#diff-67c494d6d292cbe0a96d420fa02cf4589c7fad5a8d9abbdfa317d49a8bdce6daR1305

I kind of want to get rid of that function, the few that are in that list are there because it was too painful to migrate otherwise

could you solve it properly?

could you solve it properly?

Fully getting rid of that function is going to be quite an effort, because it is in impl AttributeExt for Attribute, which is relied on by many parts of the compiler.

I've started the process (#142552) of migrating from Attribute to either AttrItem or AttributeKind in other parts of the compiler, which should help both with getting rid of that impl specifically and with other #131229-related changes in general.

While this PR can technically be fixed independently of #142552, it will cause merge conflicts, so I'm going to wait for that one first instead.

no, I mean that check_attr doesn't rely on .span() for rustc_as_ptr

compiler/rustc_attr_parsing/src/attributes/markers.rs

compiler/rustc_attr_parsing/src/attributes/mod.rs

compiler/rustc_attr_data_structures/src/attributes.rs

jdonszelmann · 2025-06-14T14:32:00Z

a single impl parsing all such attributes

I agree with @mejrs here, all attributes should get their own parser. Only if a parsed hir attribute's diagnostics depends on more than one syntactic attribute is it sensible to combine accept functions (see stability, and for example I intend to do this with allow/deny)

jdonszelmann · 2025-06-14T14:33:16Z

I do intend to make a simpler parser for single word attribute parsers just like there's already SingleAttributeParser. That transformation is trivial later, let's keep it as is for now. This change is at least conceptually good @GrigorenkoPV , just needs some minor work

jdonszelmann · 2025-06-14T14:34:51Z

@rustbot author

rustbot · 2025-06-14T14:34:55Z

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

GrigorenkoPV · 2025-06-14T17:28:59Z

@rustbot ready

jdonszelmann · 2025-06-15T11:04:41Z

@rustbot author

GrigorenkoPV · 2025-06-15T11:29:04Z

@rustbot ready

jdonszelmann · 2025-06-15T14:45:19Z

@rustbot author

jdonszelmann · 2025-06-16T15:04:52Z

compiler/rustc_attr_parsing/src/attributes/lint_helpers.rs

+
+    fn convert(_cx: &mut AcceptContext<'_, '_, S>, _args: &ArgParser<'_>) -> Option<AttributeKind> {
+        // FIXME: check that there's no args
+        Some(AttributeKind::AsPtr(_cx.attr_span))


jdonszelmann · 2025-06-16T15:05:35Z

looks good to me after that, r=me after CI passes and that's fixed

jdonszelmann · 2025-06-16T16:46:47Z

@bors r+ rollup

bors · 2025-06-16T16:46:51Z

📌 Commit da8d6bb has been approved by jdonszelmann

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - #139340 (Fix RISC-V C function ABI when passing/returning structs containing floats) - #142341 (Don't suggest converting `///` to `//` when expecting `,`) - #142414 (ignore `run-make` tests that need `std` on targets without `std`) - #142498 (Port `#[rustc_as_ptr]` to the new attribute system) - #142554 (Fix `PathSource` lifetimes.) - #142562 (Update the `backtrace` submodule) - #142565 (Test naked asm for wasm32-unknown-unknown) - #142573 (`fn candidate_is_applicable` to method) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #142498 - GrigorenkoPV:as-ptr-refactor, r=jdonszelmann Port `#[rustc_as_ptr]` to the new attribute system It might make sense to introduce some new parser analogous to `Single`, but even more simple: for parsing attributes that take no arguments and may appear only once (such as `#[rustc_as_ptr]` or `#[rustc_const_stable_indirect]`). Not sure if this should be a single `impl` parsing all such attributes, or one impl per attribute. Or how it will play along with the upcoming rework of attribute validation. Or how these argumentless attributes should be called (I've loosely referred to them as `markers` in the name of the new module in this PR, but not sure how good it is). This is a part of #131229, so r? `@jdonszelmann` --- For reference, the `#[rustc_as_ptr]` attribute was created back in #132732 as a followup to #128985.

rustbot assigned jdonszelmann Jun 14, 2025

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 Jun 14, 2025

mejrs reviewed Jun 14, 2025

View reviewed changes

compiler/rustc_attr_parsing/src/attributes/markers.rs Outdated Show resolved Hide resolved

compiler/rustc_attr_parsing/src/attributes/mod.rs Outdated Show resolved Hide resolved

jdonszelmann reviewed Jun 14, 2025

View reviewed changes

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 Jun 14, 2025

GrigorenkoPV force-pushed the as-ptr-refactor branch from 21f71d7 to 86039cc Compare June 14, 2025 14:57

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 Jun 14, 2025

jdonszelmann mentioned this pull request Jun 15, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

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 Jun 15, 2025

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 Jun 15, 2025

GrigorenkoPV mentioned this pull request Jun 15, 2025

Port #[may_dangle] to the new attribute system #142539

Open

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 Jun 15, 2025

GrigorenkoPV mentioned this pull request Jun 15, 2025

Use AttrItem instead of Attribute in more places #142552

Open

rustbot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 15, 2025

GrigorenkoPV force-pushed the as-ptr-refactor branch 2 times, most recently from aec31cf to ff6ed54 Compare June 16, 2025 14:19

jdonszelmann reviewed Jun 16, 2025

View reviewed changes

Port #[rustc_as_ptr] to the new attribute system

da8d6bb

GrigorenkoPV force-pushed the as-ptr-refactor branch from ff6ed54 to da8d6bb Compare June 16, 2025 15:13

Kobzol mentioned this pull request Jun 16, 2025

Rollup of 8 pull requests #142589

Merged

bors merged commit 1e950ef into rust-lang:master Jun 16, 2025
10 checks passed

rustbot added this to the 1.89.0 milestone Jun 16, 2025

GrigorenkoPV deleted the as-ptr-refactor branch June 16, 2025 21:27

GrigorenkoPV mentioned this pull request Jun 16, 2025

Port #[rustc_pub_transparent] to the new attribute system #142600

Open

Port #[rustc_as_ptr] to the new attribute system #142498

Port #[rustc_as_ptr] to the new attribute system #142498

Conversation

GrigorenkoPV commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 14, 2025

Uh oh!

mejrs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdonszelmann commented Jun 14, 2025

Uh oh!

jdonszelmann commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdonszelmann commented Jun 14, 2025

Uh oh!

rustbot commented Jun 14, 2025

Uh oh!

GrigorenkoPV commented Jun 14, 2025

Uh oh!

jdonszelmann commented Jun 15, 2025

Uh oh!

GrigorenkoPV commented Jun 15, 2025

Uh oh!

jdonszelmann commented Jun 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdonszelmann commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdonszelmann commented Jun 16, 2025

Uh oh!

bors commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Port `#[rustc_as_ptr]` to the new attribute system #142498

Port `#[rustc_as_ptr]` to the new attribute system #142498

GrigorenkoPV commented Jun 14, 2025 •

edited

Loading

jdonszelmann Jun 16, 2025 •

edited

Loading

jdonszelmann commented Jun 14, 2025 •

edited

Loading

jdonszelmann commented Jun 16, 2025 •

edited

Loading