Port `#[rustc_skip_during_method_dispatch]` to the new attribute system #142695

GrigorenkoPV · 2025-06-18T21:36:21Z

Part of #131229

r? @jdonszelmann

rustbot · 2025-06-18T21:36:25Z

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

rustbot · 2025-06-18T21:36:28Z

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

jdonszelmann · 2025-06-18T21:49:02Z

compiler/rustc_hir_analysis/src/collect.rs

@@ -1093,22 +1094,11 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
    let rustc_coinductive = tcx.has_attr(def_id, sym::rustc_coinductive);
    let is_fundamental = tcx.has_attr(def_id, sym::fundamental);

-    // FIXME: We could probably do way better attribute validation here.


Hehe, fixed the fixme :)

jdonszelmann · 2025-06-18T21:53:23Z

compiler/rustc_attr_parsing/src/attributes/mod.rs

@@ -33,6 +33,7 @@ pub(crate) mod deprecation;
 pub(crate) mod inline;
 pub(crate) mod lint_helpers;
 pub(crate) mod repr;
+pub(crate) mod resolution;


Do we expect more parsers in this module?

Probably not, but didn't know where else to put plus in the future it might get reworked a bit if rust-lang/libs-team#605 gets accepted.

jdonszelmann · 2025-06-18T21:53:42Z

compiler/rustc_attr_parsing/src/attributes/resolution.rs

+pub(crate) struct SkipDuringMethodDispatchParser;
+
+impl<S: Stage> SingleAttributeParser<S> for SkipDuringMethodDispatchParser {
+    const PATH: &'static [Symbol] = &[sym::rustc_skip_during_method_dispatch];


I believe the 'static is unnecessary here

jdonszelmann · 2025-06-18T21:55:12Z

compiler/rustc_attr_parsing/src/attributes/resolution.rs

+    const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepFirst;
+    const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;
+
+    const TEMPLATE: AttributeTemplate = template!(List: "array, boxed_slice");


I wish we had a better way to say in templates that it's one or more of these. I think I'll at some point have a pass over all the templates we have tbh... Not related to your pr

jdonszelmann · 2025-06-18T21:57:01Z

tests/ui/attributes/rustc_skip_during_method_dispatch.stderr

+error: malformed `rustc_skip_during_method_dispatch` attribute input
+  --> $DIR/rustc_skip_during_method_dispatch.rs:3:1
+   |
+LL | #[rustc_skip_during_method_dispatch]


Error on line 3 emitted twice. You should add an exception in rustc_builtin_attrs to no longer validate this attribute. That exception will be temporary until all attrs are migrated. Take a look at what I did for the inline parser (search for sym::inline in the compiler what will also show you it).

GrigorenkoPV · 2025-06-18T22:12:12Z

@rustbot ready

Bleachfuel · 2025-06-18T23:17:32Z

compiler/rustc_attr_parsing/src/attributes/resolution.rs

+                ArgParser::NameValue(args) => Some(args.eq_span.to(args.value_span)),
+            } {
+                cx.unexpected_literal(span);
+            }


U can use ArgParser::no_args

Bleachfuel · 2025-06-18T23:19:29Z

compiler/rustc_attr_parsing/src/attributes/resolution.rs

+            if let Some(span) = match arg.args() {
+                ArgParser::NoArgs => None,
+                ArgParser::List(args) => Some(args.span),
+                ArgParser::NameValue(args) => Some(args.eq_span.to(args.value_span)),
+            } {
+                cx.unexpected_literal(span);
+            }


Suggested change

if let Some(span) = match arg.args() {

ArgParser::NoArgs => None,

ArgParser::List(args) => Some(args.span),

ArgParser::NameValue(args) => Some(args.eq_span.to(args.value_span)),

} {

cx.unexpected_literal(span);

}

if !arg.args().no_args() {

cx.unexpected_literal(span);

}

This will give a less precise span. I'm planning to move this code into a helper function in a follow-up PR. Or maybe I can do it in this one.

Wasn't there a .span() method to do this combining already? Saying that from memory, didn't check

Wasn't there a .span() method to do this combining already? Saying that from memory, didn't check

There was, but it returns an Option<Span> where None <=> NoArgs, but I didn't want to rely on that implementation detail.

This will give a less precise span. I'm planning to move this code into a helper function in a follow-up PR. Or maybe I can do it in this one.

Done.

bors · 2025-06-19T07:29:11Z

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

jdonszelmann · 2025-06-20T06:13:32Z

compiler/rustc_attr_parsing/src/context.rs

@@ -310,6 +322,24 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
            },
        })
    }
+
+    pub(crate) fn expect_no_args(&self, args: &ArgParser<'_>) -> Result<(), ErrorGuaranteed> {
+        if let Some(span) = match args {


I think for #[cold] I implemented this exact method. Could you rebase on that branch? Unfortunately it's taking a bit for everything to merge in. #142491

bors · 2025-06-20T14:00:15Z

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

rustbot assigned jdonszelmann Jun 18, 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 18, 2025

jdonszelmann requested changes Jun 18, 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 18, 2025

GrigorenkoPV force-pushed the attributes/rustc_skip_during_method_dispatch branch from 84a4a9a to e015bc7 Compare June 18, 2025 22:11

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 18, 2025

Bleachfuel reviewed Jun 18, 2025

View reviewed changes

jdonszelmann mentioned this pull request Jun 19, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

GrigorenkoPV added 2 commits June 19, 2025 18:12

Port #[rustc_skip_during_method_dispatch] to the new attribute system

b431b40

Add a helper expect_no_args method

961a2cd

GrigorenkoPV force-pushed the attributes/rustc_skip_during_method_dispatch branch from e015bc7 to 961a2cd Compare June 19, 2025 15:29

jdonszelmann reviewed Jun 20, 2025

View reviewed changes

Port #[rustc_skip_during_method_dispatch] to the new attribute system #142695

Are you sure you want to change the base?

Port #[rustc_skip_during_method_dispatch] to the new attribute system #142695

Conversation

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

Uh oh!

Uh oh!

rustbot commented Jun 18, 2025

Uh oh!

rustbot commented Jun 18, 2025

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!

GrigorenkoPV commented Jun 18, 2025

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 19, 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!

Choose a reason for hiding this comment

Uh oh!

bors commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jun 20, 2025

Uh oh!

Uh oh!

Port `#[rustc_skip_during_method_dispatch]` to the new attribute system #142695

Port `#[rustc_skip_during_method_dispatch]` to the new attribute system #142695

GrigorenkoPV commented Jun 18, 2025 •

edited by rustbot

Loading

jdonszelmann Jun 19, 2025 •

edited

Loading