Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix ICE on stable related to attrs on macros
  • Loading branch information
jdonszelmann committed Aug 24, 2025
commit 48a4e2d2dde8d68e1d00d3eac07b2c6155f3239d
21 changes: 2 additions & 19 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl Stage for Early {
sess: &'sess Session,
diag: impl for<'x> Diagnostic<'x>,
) -> ErrorGuaranteed {
self.should_emit().emit_err_or_delay(sess.dcx().create_err(diag))
self.should_emit().emit_err(sess.dcx().create_err(diag))
}

fn should_emit(&self) -> ShouldEmit {
Expand Down Expand Up @@ -667,29 +667,12 @@ pub enum ShouldEmit {
}

impl ShouldEmit {
pub(crate) fn emit_err_or_delay(&self, diag: Diag<'_>) -> ErrorGuaranteed {
pub(crate) fn emit_err(&self, diag: Diag<'_>) -> ErrorGuaranteed {
match self {
ShouldEmit::EarlyFatal if diag.level() == Level::DelayedBug => diag.emit(),
ShouldEmit::EarlyFatal => diag.upgrade_to_fatal().emit(),
ShouldEmit::ErrorsAndLints => diag.emit(),
ShouldEmit::Nothing => diag.delay_as_bug(),
}
}

pub(crate) fn maybe_emit_err(&self, diag: Diag<'_>) {
match self {
ShouldEmit::EarlyFatal if diag.level() == Level::DelayedBug => {
diag.emit();
}
ShouldEmit::EarlyFatal => {
diag.upgrade_to_fatal().emit();
}
ShouldEmit::ErrorsAndLints => {
diag.emit();
}
ShouldEmit::Nothing => {
diag.cancel();
}
}
}
}
20 changes: 10 additions & 10 deletions compiler/rustc_attr_parsing/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,14 @@ fn expr_to_lit(
match res {
Ok(lit) => {
if token_lit.suffix.is_some() {
should_emit.emit_err_or_delay(
should_emit.emit_err(
psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }),
);
None
} else {
if !lit.kind.is_unsuffixed() {
// Emit error and continue, we can still parse the attribute as if the suffix isn't there
should_emit.maybe_emit_err(
should_emit.emit_err(
psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }),
);
}
Expand All @@ -355,19 +355,19 @@ fn expr_to_lit(
}
}
} else {
if matches!(should_emit, ShouldEmit::Nothing) {
return None;
}

// Example cases:
// - `#[foo = 1+1]`: results in `ast::ExprKind::BinOp`.
// - `#[foo = include_str!("nonexistent-file.rs")]`:
// results in `ast::ExprKind::Err`. In that case we delay
// the error because an earlier error will have already
// been reported.
let msg = "attribute value must be a literal";
let mut err = psess.dcx().struct_span_err(span, msg);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a subtle bug in this version, fixed in my last commit. When an expression in an attribute is a macro, and the macro expands to a literal, and we parse an attribute with ShouldEmit::Nothing, we would delay that as a bug expecting a second parse to emit the error. However, when the macro then expands to a literal, we don't hit the same codepath and we never emit an error about this. The delayed bug turns into an ICE.

A very similar problem was happening in February already (#137687) which was recognized and even a backport was made for it. However, in later versions it still made it to stable.

The last commit of this PR adds tests for these cases and fixes this by, when we expect a literal, but have ShouldEmit::Nothing, we don't delay a bug, instead we do nothing. This is correct because if the macro indeed turns into a literal, the second parse might very well be correct. We shouldn't have delayed a bug at all.

Specifically for crate_name, we parse with ShouldEmit::EarlyFatal so when it cares about this being a literal, it will error about it as it should.

if let ExprKind::Err(_) = expr.kind {
err.downgrade_to_delayed_bug();
}

should_emit.emit_err_or_delay(err);
let err = psess.dcx().struct_span_err(span, msg);
should_emit.emit_err(err);
None
}
}
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {

if !lit.kind.is_unsuffixed() {
// Emit error and continue, we can still parse the attribute as if the suffix isn't there
self.should_emit.maybe_emit_err(
self.should_emit.emit_err(
self.parser.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }),
);
}
Expand Down Expand Up @@ -542,7 +542,7 @@ impl<'a> MetaItemListParser<'a> {
) {
Ok(s) => Some(s),
Err(e) => {
should_emit.emit_err_or_delay(e);
should_emit.emit_err(e);
None
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//@ check-pass
#[deprecated = concat !()]
Copy link
Contributor Author

@jdonszelmann jdonszelmann Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression test for a new case of #137687

Copy link
Contributor Author

@jdonszelmann jdonszelmann Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macro_rules! a {
() => {};
}
fn main() {}
9 changes: 9 additions & 0 deletions tests/ui/lint/unused/concat-in-crate-name-issue-137687.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![deny(unused)]

#[crate_name = concat !()]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//~^ ERROR crate-level attribute should be an inner attribute: add an exclamation mark: `#![foo]
macro_rules! a {
//~^ ERROR unused macro definition
() => {};
}
fn main() {}
23 changes: 23 additions & 0 deletions tests/ui/lint/unused/concat-in-crate-name-issue-137687.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: unused macro definition: `a`
--> $DIR/concat-in-crate-name-issue-137687.rs:5:14
|
LL | macro_rules! a {
| ^
|
note: the lint level is defined here
--> $DIR/concat-in-crate-name-issue-137687.rs:1:9
|
LL | #![deny(unused)]
| ^^^^^^
= note: `#[deny(unused_macros)]` implied by `#[deny(unused)]`

error: crate-level attribute should be an inner attribute: add an exclamation mark: `#![foo]`
--> $DIR/concat-in-crate-name-issue-137687.rs:3:1
|
LL | #[crate_name = concat !()]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(unused_attributes)]` implied by `#[deny(unused)]`

error: aborting due to 2 previous errors

1 change: 1 addition & 0 deletions tests/ui/resolve/path-attr-in-const-block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ fn main() {
const {
#![path = foo!()]
//~^ ERROR: cannot find macro `foo` in this scope
//~| ERROR: attribute value must be a literal
}
}
8 changes: 7 additions & 1 deletion tests/ui/resolve/path-attr-in-const-block.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,11 @@ error: cannot find macro `foo` in this scope
LL | #![path = foo!()]
| ^^^

error: aborting due to 1 previous error
error: attribute value must be a literal
--> $DIR/path-attr-in-const-block.rs:6:19
|
LL | #![path = foo!()]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra error, but this is correct/expected. bonus!

| ^^^^^^

error: aborting due to 2 previous errors

Loading