add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

jdonszelmann · 2025-02-27T13:57:10Z

compiler/rustc_driver_impl/src/lib.rs

fmease · 2025-03-06T09:02:56Z

compiler/rustc_attr_data_structures/src/attributes.rs

+        name: Symbol,
+        name_span: Span,


This could theoretically use Spanned but it doesn't necessarily lead to more readable code (and I don't remember if it's compatible with translatable diagnostics).

Suggested change

name: Symbol,

name_span: Span,

name: Symbol,

name_span: Span,

hmm, I'm not sure that makes it that much better but if you really want it I'm fine with it. Other attrs sometimes have more than 1 span related and then it's nice to distinguish them by name but here it's possible

fmease · 2025-03-06T09:07:15Z

compiler/rustc_attr_parsing/src/attributes/crate_name.rs

+pub(crate) struct CratenameParser;
+
+impl SingleAttributeParser for CratenameParser {
+    const PATH: &'static [Symbol] = &[sym::crate_name];


FYI: You generally no longer need to explicitly state 'static in the ty of assoc consts.

fair enough, d'ya mind if I remove that for all attr parsers in another PR? they'll all have this

fmease · 2025-03-06T09:12:17Z

compiler/rustc_attr_parsing/src/attributes/crate_name.rs

+
+    fn on_duplicate(cx: &AcceptContext<'_>, first_span: Span) {
+        // FIXME(jdonszelmann): better duplicate reporting (WIP)
+        cx.emit_err(UnusedMultiple {


Duplicates are warnings on master, right? So this is a breaking change?

jdonszelmann · 2025-03-06T16:12:47Z

@fmease and I decided to delay this PR for a bit until more diagnostic infra for attrs lands (hopefully next week) to make the duplicate error a warning to be consistent with current master.

fmease · 2025-03-06T16:14:30Z

r=fmease,bjorn3 once that PR lands and you can downgrade the error to a lint warning again.

@bors delegate+

bors · 2025-03-06T16:14:34Z

✌️ @jdonszelmann, you can now approve this pull request!

If @fmease told you to "r=me" after making some further change, please make that change, then do @bors r=@fmease

bors · 2025-03-13T05:36:17Z

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

wesleywiser · 2025-04-24T14:46:44Z

Hi @jdonszelmann and @fmease, #137687 has reached beta so it would be great if we could resolve that via a beta-backport of this PR. Do you think it's possible to resolve the review feedback and merge this PR in the next week or so?

I can probably pitch in to help if necessary 🙂

jdonszelmann · 2025-04-24T14:50:11Z

Hi there. We wanted to delay this until attribute diagnostics. Something that should not be a lot of work but I have not worked as much on recently as I maybe should. I can't to this tomorrow, but likely next week. The pr has been reviewed but there was a bit of an issue around incremental, which I now know how to solve but just haven't yet. Is that a reasonable timeframe? I am pretty busy with organising rust week atm, but knowing the solution I guess the only important thing is that it gets quick reviews? Let me know how that sounds

wesleywiser · 2025-04-24T14:53:12Z

Just to clarify, will this PR then depend on the attribute diagnostics work? (Is that #138164?)

Generally, for backports, we try to keep the backport as small as possible so if this PR can be made to work independently of that PR (possibly even with a degraded diagnostics experience), that would be highly preferred.

jdonszelmann · 2025-04-24T16:28:00Z

No. We decided to delay this because it's relatively low prio and any nice and permanent solution for this depends on that. I'll make a temp fix that at least doesn't ice separately then, that sounds better for now. I don't mind doing it, I know the problem now and then I also know how to fix it permanently later. You'll hear from me

theemathas · 2025-05-01T14:19:57Z

Note that #137687 actually has two separate problems/subissues:

A problem (lower priority) involving the #[crate_name] attribute.
A problem (higher priority, found in a crater run) involving user-defined macro_rules and derive macros.

This PR only appears to address the first problem. The second problem would probably need to have a separate fix.

jdonszelmann · 2025-05-02T13:23:01Z

ok this is weird, I've copied just the tests over to master to fix it in a temporary way, but can no longer reporoduce the issue. Something else must have solved it in the meantime. Let me see what.

jieyouxu · 2025-05-03T09:36:54Z

(I know I've said this in the other PR too, but just in case)

Just a heads up: I split #137687 into two issues:

ICE: expr in place where literal is expected (builtin attr parsing) #137687 is used to track Matthias's fuzzed regression (the #[crate_name = concat!("Cloneb")] one).
regression: ICE: expr in place where literal is expected (builtin attr parsing) #140612 is used to track the crater regressions (known to affect user macros)

jdonszelmann · 2025-08-11T12:52:44Z

@fmease finally got around to this one. The 2nd commit introduces lints for early-parsed attributes which was something that was unsupported earlier simply because no attribute needed it until now. The warning/error is unfortunately emitted twice now which is hard to fix in this PR. We simply call a very long chain of functions twice, and expect the first time to emit errors and the 2nd not. It's hard to distinguish these cases, and in some cases when we try to get metadata from rustc we only actually trigger the chain once so it's hard to even define in which call to emit errors. I left a todo for it.

jdonszelmann · 2025-08-11T12:52:57Z

should merge after #145243 which is a PR that does some generic diagnostics work that impacts this PR a lot.

fmease

I have some nits and questions but generally 👍, thanks!

compiler/rustc_hir/src/attrs/encode_cross_crate.rs

compiler/rustc_interface/src/passes.rs

compiler/rustc_interface/src/util.rs

compiler/rustc_lint/src/early/diagnostics.rs

fmease · 2025-08-11T16:52:01Z

tests/ui/attributes/malformed-attrs.stderr

+  --> $DIR/malformed-attrs.rs:71:1
+   |
+LL | #[crate_name]
+   | ^^^^^^^^^^^^^ help: must be of the form: `#[crate_name = "name"]`


(will be must be of the form: `#![crate_name = "name"]` (extra !) after #145238 (future conflict))

I think it's after this one #145243 but ye

fmease · 2025-08-11T16:58:18Z

tests/ui/invalid-compile-flags/print-crate-name-request-malformed-crate-name.stdout

@@ -0,0 +1 @@
+print_crate_name_request_malformed_crate_name


Ah, interesting ^^. This'll lead to some interesting output in the terminal where stdout & stderr are typically merged.

Ideally we shouldn't emit this (wrong) crate name … but it's probably unavoidable rn since the new attr parsing APIs don't return a Result<_, _>?

hmm, yea if the attr can't be parsed it's as if the crate name wasn't set at all. Hard to change since that's usually the behavior we want, i.e. give an error, ignore it, and continue

fmease · 2025-08-11T17:03:12Z

compiler/rustc_attr_parsing/src/context.rs

        };
        let mut parsed = p.parse_attribute_list(
            attrs,
            target_span,
            target_node_id,
            OmitDoc::Skip,
            std::convert::identity,
-            |_lint| {
-                panic!("can't emit lints here for now (nothing uses this atm)");
+            |AttributeLint { id, span, kind }| {


(Since I lack a bigger picture of the new attr parsing APIs) Is it desirable that parse_limited{,_should_emit} default to lint buffering by default? Shouldn't the caller decide that (via a callback)?

For example, what happens if someone calls one of these functions in a later compiler stage when all buffered lints "have been emitted already"? Can that even happen or are buffered lints only emitted when the session is destroyed (I don't know much about this logic of buffer_lint)?

that can't really happen

in later compiler stages you don't have ast::Attribute's anymore to parse since they're all parsed during lowering. This is just for the few cases where we want to parse earlier. The only way this could go wrong in is during rustc_ast_lowering where you do still have ast attributes but have already emitted all lints. However, this is also the place where we do all existing attr parsing already in the right way. It's hard to mess that up there.

compiler/rustc_middle/src/ty/context.rs

jdonszelmann · 2025-08-12T10:01:12Z

@rustbot review

rustbot assigned fmease Feb 27, 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 Feb 27, 2025

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the fix-137687 branch from 04bafec to 3898182 Compare February 27, 2025 14:07

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the fix-137687 branch from 3898182 to 81fc9d1 Compare February 27, 2025 14:39

bjorn3 reviewed Feb 27, 2025

View reviewed changes

compiler/rustc_driver_impl/src/lib.rs Outdated Show resolved Hide resolved

jdonszelmann force-pushed the fix-137687 branch 2 times, most recently from 4e90cee to 21838d5 Compare February 27, 2025 16:51

bjorn3 reviewed Feb 27, 2025

View reviewed changes

compiler/rustc_driver_impl/src/lib.rs Outdated Show resolved Hide resolved

jdonszelmann force-pushed the fix-137687 branch from 21838d5 to d1ee9c4 Compare February 27, 2025 17:44

fmease approved these changes Mar 6, 2025

View reviewed changes

fmease added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2025

fmease mentioned this pull request May 2, 2025

add test to reproduce #137687 and add a hotfix #140584

Closed

jdonszelmann mentioned this pull request Jun 13, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

jdonszelmann force-pushed the fix-137687 branch from d1ee9c4 to 1f5c49f Compare August 11, 2025 12:50

fmease approved these changes Aug 11, 2025

View reviewed changes

fmease reviewed Aug 11, 2025

View reviewed changes

compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved

jdonszelmann force-pushed the fix-137687 branch from 1f5c49f to a584056 Compare August 12, 2025 10:00

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 12, 2025

jdonszelmann added 2 commits August 12, 2025 12:15

take attr style into account in diagnostics

68d388f

Port crate name to the new attribute system

86f64ca

jdonszelmann force-pushed the fix-137687 branch from a584056 to 86f64ca Compare August 12, 2025 10:15

		@@ -0,0 +1 @@
		print_crate_name_request_malformed_crate_name

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Are you sure you want to change the base?

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Conversation

jdonszelmann commented Feb 27, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

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!

jdonszelmann commented Mar 6, 2025

Uh oh!

fmease commented Mar 6, 2025

Uh oh!

bors commented Mar 6, 2025

Uh oh!

bors commented Mar 13, 2025

Uh oh!

wesleywiser commented Apr 24, 2025

Uh oh!

jdonszelmann commented Apr 24, 2025

Uh oh!

wesleywiser commented Apr 24, 2025

Uh oh!

jdonszelmann commented Apr 24, 2025

Uh oh!

theemathas commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

jieyouxu commented May 3, 2025

Uh oh!

jdonszelmann commented Aug 11, 2025

Uh oh!

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

Uh oh!

Uh oh!

fmease left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 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!

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!

Uh oh!

jdonszelmann commented Aug 12, 2025

Uh oh!

Uh oh!

add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

theemathas commented May 1, 2025 •

edited

Loading

jdonszelmann commented May 2, 2025 •

edited

Loading

jdonszelmann commented Aug 11, 2025 •

edited

Loading

fmease Aug 11, 2025 •

edited

Loading