Fix ICE when applying test macro to crate root #147841

jdonszelmann · 2025-10-18T10:28:10Z

This PR does a couple of things. First of all, I found an ICE that happens when applying #![core::prelude::v1::test] to the crate root. This is caused by the test macro not expanding to an item when --test isn't applied. For the crate root, that means it deletes the crate....

The fix now first does target checking, and only if the target is valid discards the item when --test isn't applied. The discarding is, I think, important for perf.

The problem with this PR is that it means that #[test] applied to structs previously would give no errors unless --test is applied! That sounds like a bug to me, but maybe we should crater run it just in case, since technically that's a breaking change. Errors in such items wouldn't be reported previously.

Also fixed a smol diagnostics bug with #[bench]'s error messages refering to #[test] accidentally.

r? noratrieb (since I already explained you a bunch, feel free to re-assign)

Fixes #114920

rustbot · 2025-10-18T10:28:14Z

noratrieb is not on the review rotation at the moment.
They may take a while to respond.

jdonszelmann · 2025-10-18T10:29:07Z

compiler/rustc_builtin_macros/src/test.rs

+
+    if !is_bench {
+        err.with_span_suggestion(attr_sp,
            "replace with conditional compilation to make the item only exist when tests are being run",


only give the suggestion of cfg(test) when we're dealing with test attrs

jdonszelmann · 2025-10-18T10:29:39Z

tests/ui/feature-gates/gating-of-test-attrs.rs

@@ -0,0 +1,48 @@
+#![feature(test)]
+
+// test is a built-in macro, not a built-in attribute, but it kind of acts like both.


New test for test attributes. The previous tests simply asserted no errors would be given but that's not true anymore, and I think they should error!

jdonszelmann · 2025-10-18T10:30:01Z

tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.rs

-// non-crate-level #[test] attributes seem to be ignored.
-
-#[test]
-mod test { mod inner { #![test] }


these were weird here anyway, test isn't a built-in attribute but a built-in macro!

jdonszelmann · 2025-10-18T10:30:39Z

tests/ui/macros/issue-111749.rs

@@ -5,8 +5,8 @@ macro_rules! cbor_map {
 }

 fn main() {
-    cbor_map! { #[test(test)] 4};
-    //~^ ERROR removing an expression is not supported in this position
+    cbor_map! { #[test(test)] 4i32};


changed to an i32 cause signum wouldn't be valid on just 4

jdonszelmann · 2025-10-18T10:31:26Z

tests/ui/macros/issue-111749.rs

-    cbor_map! { #[test(test)] 4};
-    //~^ ERROR removing an expression is not supported in this position
+    cbor_map! { #[test(test)] 4i32};
+    //~^ ERROR the `#[test]` attribute may only be used on a non-associated function


I think this is the correct error here, not something about removing expressions. I think noting that the target is wrong is more important.

jdonszelmann · 2025-10-18T10:31:45Z

tests/ui/macros/test-on-crate-root.rs

@@ -0,0 +1,8 @@
+// ICE when applying `#![test]` to the crate root,
+// though only when specified with a full path. `#![test]` is not enough.
+#![core::prelude::v1::test]


tests the specific ICE from the PR description

jdonszelmann · 2025-10-18T10:33:12Z

tests/ui/macros/attr-empty-expr.stderr

-LL |     let _ = #[test] 0;
-   |             ^^^^^^^
-
-error: removing an expression is not supported in this position


this test was testing for "empty expressions". this is super niche, can only happen with ast-based attributes (only like 10 in the compiler) and none of the examples in this test should actually remove the expression, they should instead report that the target is wrong. I can't find any other examples of node-removing attributes so I think this test is simply superfluous now.

jdonszelmann · 2025-10-18T10:34:22Z

@bors try
@craterbot check

craterbot · 2025-10-18T10:34:24Z

🚨 Error: missing start toolchain

🆘 If you have any trouble with Crater please ask in t-infra on Zulip
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Fix ICE when applying test macro to crate root

matthiaskrgr · 2025-10-18T10:58:11Z

does this fix #114920 ?

jdonszelmann · 2025-10-18T11:10:10Z

yea, that's actually exactly what it fixes. there was a crashtest for it I just notice but the fix is rather trivial in the end

jdonszelmann · 2025-10-18T11:12:41Z

@bors try cancel

rust-bors · 2025-10-18T11:12:44Z

Try build cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/18614510844

rustbot · 2025-10-18T11:14:19Z

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

jdonszelmann · 2025-10-18T11:14:25Z

@bors try

Fix ICE when applying test macro to crate root

rust-bors · 2025-10-18T13:25:55Z

☀️ Try build successful (CI)
Build commit: 73ffebf (73ffebff6f34db7eb167bf9edcf2afa19c3073e9, parent: ab1d2444533d829e2d5cff6634cd3c70de6d7103)

jdonszelmann · 2025-10-18T14:11:58Z

@craterbot check

craterbot · 2025-10-18T14:12:30Z

👌 Experiment pr-147841 created and queued.
🤖 Automatically detected try build 73ffebf
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

jdonszelmann · 2025-10-20T09:02:03Z

@rustbot review (@Noratrieb?)

Noratrieb · 2025-10-28T18:26:14Z

do you think it would make sense to get quick lang team signoff here? given that it already errors in test mode I'm not sure whether we should treat it as breakage or not

jdonszelmann · 2025-10-28T21:55:32Z

sure, let's do it. cc @rust-lang/lang

Noratrieb · 2025-10-28T22:41:52Z

relevant part of the description

The fix now first does target checking, and only if the target is valid discards the item when --test isn't applied. The discarding is, I think, important for perf.

The problem with this PR is that it means that #[test] applied to structs previously would give no errors unless --test is applied! That sounds like a bug to me, but maybe we should crater run it just in case, since technically that's a breaking change. Errors in such items wouldn't be reported previously.

From the description. Should either be an easy FCP or a decision to not even FCP, given that test mode already errored.

tmandry · 2025-10-29T17:52:39Z

tests/ui/feature-gates/gating-of-test-attrs.rs

+// test is a built-in macro, not a built-in attribute, but it kind of acts like both.
+// check its target checking anyway here
+#[test]
+//~^ ERROR the `#[test]` attribute may only be used on a non-associated function


This is inconsistent with the terminology we use in the reference and elsewhere.

Suggested change

//~^ ERROR the `#[test]` attribute may only be used on a non-associated function

//~^ ERROR the `#[test]` attribute may only be used on a free function

I'm not sure I like that convention, but I do agree with wanting it to be consistent. Changed it :)

traviscross · 2025-10-29T17:52:41Z

Sounds right. Thanks @jdonszelmann, @Noratrieb.

This brings us closer in line with what the Reference says in:

https://doc.rust-lang.org/1.90.0/reference/attributes/testing.html#r-attributes.testing.test.allowed-positions

@rfcbot fcp merge

rust-rfcbot · 2025-10-29T17:52:43Z

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

tmandry · 2025-10-29T17:53:15Z

@rfcbot reviewed

nikomatsakis · 2025-10-29T17:53:56Z

@rfcbot reviewed

rust-rfcbot · 2025-10-29T17:54:05Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rustbot · 2025-10-30T09:48:40Z

This PR was rebased onto a different master 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.

rustbot assigned Noratrieb Oct 18, 2025

rustbot added 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 Oct 18, 2025

jdonszelmann commented Oct 18, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 18, 2025

Auto merge of #147841 - jdonszelmann:test-macro-ice, r=<try>

013fb92

Fix ICE when applying test macro to crate root

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the test-macro-ice branch from 3e897fe to a9e4630 Compare October 18, 2025 11:14

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 18, 2025

Auto merge of #147841 - jdonszelmann:test-macro-ice, r=<try>

73ffebf

Fix ICE when applying test macro to crate root

jdonszelmann mentioned this pull request Oct 18, 2025

Fix ICE from invalid delimeter in attrs parsing #147778

Closed

craterbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2025

ICE when applying test to crate root

108ebe3

tmandry reviewed Oct 29, 2025

View reviewed changes

rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 29, 2025

rust-rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 29, 2025

only discard items with #[test] on it when target is valid

cec2d48

jdonszelmann force-pushed the test-macro-ice branch from a9e4630 to 98703b0 Compare October 30, 2025 09:48

fixup name in diagnostics

5aa296b

jdonszelmann force-pushed the test-macro-ice branch from 98703b0 to 5aa296b Compare October 30, 2025 09:51

		@@ -0,0 +1,48 @@
		#![feature(test)]

		// test is a built-in macro, not a built-in attribute, but it kind of acts like both.

	//~^ ERROR the `#[test]` attribute may only be used on a non-associated function
	//~^ ERROR the `#[test]` attribute may only be used on a free function

Fix ICE when applying test macro to crate root #147841

Are you sure you want to change the base?

Fix ICE when applying test macro to crate root #147841

Uh oh!

Conversation

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

Uh oh!

Uh oh!

rustbot commented Oct 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!

Choose a reason for hiding this comment

Uh oh!

jdonszelmann commented Oct 18, 2025

Uh oh!

craterbot commented Oct 18, 2025

Uh oh!

This comment has been minimized.

matthiaskrgr commented Oct 18, 2025

Uh oh!

This comment has been minimized.

jdonszelmann commented Oct 18, 2025

Uh oh!

jdonszelmann commented Oct 18, 2025

Uh oh!

rust-bors bot commented Oct 18, 2025

Uh oh!

rustbot commented Oct 18, 2025

Uh oh!

jdonszelmann commented Oct 18, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Oct 18, 2025

Uh oh!

jdonszelmann commented Oct 18, 2025

Uh oh!

craterbot commented Oct 18, 2025

Uh oh!

jdonszelmann commented Oct 20, 2025

Uh oh!

Noratrieb commented Oct 28, 2025

Uh oh!

jdonszelmann commented Oct 28, 2025

Uh oh!

Noratrieb commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

traviscross commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-rfcbot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmandry commented Oct 29, 2025

Uh oh!

nikomatsakis commented Oct 29, 2025

Uh oh!

rust-rfcbot commented Oct 29, 2025

Uh oh!

rustbot commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jdonszelmann commented Oct 18, 2025 •

edited

Loading

traviscross commented Oct 29, 2025 •

edited

Loading

rust-rfcbot commented Oct 29, 2025 •

edited

Loading