drop derive helpers during attribute parsing by scrabsha · Pull Request #153540 · rust-lang/rust

scrabsha · 2026-03-07T15:18:16Z

first two commits (7db3f6e, 0033b31) move attribute target checks from rustc_passes to rustc_attr_parsing. last commit (38f62ee) actually fixes the issue.

the diagnostics slightly regressed and i'm not super happy about it, but i doubt there's much we can do to avoid that.

r? @jdonszelmann
r? @JonathanBrouwer

rustbot · 2026-03-07T15:18:20Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

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

cc @jdonszelmann, @JonathanBrouwer

compiler/rustc_attr_parsing/src/context.rs

compiler/rustc_parse/src/parser/mod.rs

compiler/rustc_attr_parsing/src/errors.rs

compiler/rustc_attr_parsing/src/interface.rs

compiler/rustc_expand/src/expand.rs

mejrs · 2026-03-07T16:41:43Z

compiler/rustc_attr_parsing/src/target_checking.rs

        allowed_targets: &AllowedTargets,
        target: Target,
        cx: &mut AcceptContext<'_, 'sess, S>,
+        first_item_in_crate: Option<Span>,


This first_item_in_crate parameter is rather ugly, especially because None is passed to it so often. I haven't tried but maybe you'd be better off just grabbing the span of the attribute and then going through the sourcemap looking for the next non-[comment, doc comment, other attribute etc] thing and pointing at that.

that's a good idea, i'll experiment on this later today

@rustbot author

This first_item_in_crate parameter is rather ugly, especially because None is passed to it so often. I haven't tried but maybe you'd be better off just grabbing the span of the attribute and then going through the sourcemap looking for the next non-[comment, doc comment, other attribute etc] thing and pointing at that.

so i did just that. it improved the oracles a lot and allowed me to completely remove the ugly first_item_in_crate which is good news!

but also, the code is far from perfect. i tried to cut corners where possible in order to keep the code readable. i can probably add support for nested multiline comments without making the code too hard to review, but i don't think there's anything i can realistically do to properly support the attributes (that would require keeping a stack of the opened delimiters, which in turn would requires a lexer).

i took the liberty of using try_blocks in order to keep the code somewhat concise (cool feature, btw!). it looks like this feature is already used in the compiler, so i figured it wouldn't hurt, but i can totally remove it if needed.

@rustbot ready

rustbot · 2026-03-08T10:47:58Z

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

tests/ui/derives/issue-36617.stderr

rustbot · 2026-03-17T20:47:38Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

JonathanBrouwer · 2026-03-20T10:50:04Z

compiler/rustc_attr_parsing/src/interface.rs

        sess: &'sess Session,
        attrs: &[ast::Attribute],
        sym: Symbol,
+        target: Target,


Why do we need to add the target to parse_limited, since this function will never emit?

JonathanBrouwer · 2026-03-20T10:54:49Z

compiler/rustc_attr_parsing/src/target_checking.rs

+                .source_map()
+                .span_to_snippet(span)
+                .ok()
+                .filter(|src| src.starts_with("#!["))


There is surely a better way to figure out if this is an inner attribute than this?
Can we look at the AcceptContext for this?

JonathanBrouwer · 2026-03-20T11:02:47Z

compiler/rustc_attr_parsing/src/interface.rs

+
+    // FIXME: Fix "Cannot determine resolution" error and remove built-in macros
+    // from this check.
+    fn check_invalid_crate_level_attr_item(&self, attr: &AttrItem) {


Can we move these functions to target_checking.rs?

JonathanBrouwer · 2026-03-20T11:05:52Z

compiler/rustc_attr_parsing/src/interface.rs

+        // which were unsuccessfully resolved due to cannot determine
+        // resolution for the attribute macro error.
+        const ATTRS_TO_CHECK: &[Symbol] =
+            &[sym::derive, sym::test, sym::test_case, sym::global_allocator, sym::bench];


How reachable is this code? Are these attributes not removed during expansion, I thought for example test was removed

rust-bors · 2026-03-20T14:35:28Z

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

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 Mar 7, 2026

rustbot assigned jdonszelmann Mar 7, 2026

mejrs reviewed Mar 7, 2026

View reviewed changes

scrabsha force-pushed the sasha/drop-derive-helpers branch from 38f62ee to 676a6d9 Compare March 8, 2026 10:29

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 Mar 8, 2026

jdonszelmann reviewed Mar 9, 2026

View reviewed changes

tests/ui/derives/issue-36617.stderr Show resolved Hide resolved

scrabsha force-pushed the sasha/drop-derive-helpers branch from 676a6d9 to 88e1141 Compare March 12, 2026 21:55

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 Mar 12, 2026

scrabsha force-pushed the sasha/drop-derive-helpers branch from 88e1141 to 3b5896d Compare March 12, 2026 22:06

This comment has been minimized.

Sign in to view

scrabsha added 3 commits March 17, 2026 20:35

check earlier for misused crate-level attributes

7e33133

check earlier for attributes that are placed in where predicate

cd4a7a1

drop derive helpers during ast lowering

0dfbffd

scrabsha force-pushed the sasha/drop-derive-helpers branch from 3b5896d to 0dfbffd Compare March 17, 2026 20:47

JonathanBrouwer self-assigned this Mar 20, 2026

JonathanBrouwer requested changes Mar 20, 2026

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 Mar 20, 2026

Uh oh!

Conversation

scrabsha commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mejrs Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

scrabsha Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

scrabsha Mar 12, 2026 • 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!

rustbot commented Mar 8, 2026

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Mar 17, 2026

Uh oh!

JonathanBrouwer Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

rust-bors bot commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

scrabsha commented Mar 7, 2026 •

edited

Loading

scrabsha Mar 12, 2026 •

edited

Loading