Skip to content

Conversation

@chenyukang
Copy link
Member

@chenyukang chenyukang commented Oct 16, 2025

Fixes #146808

I tried hard to get the correct span, but didn't come out with a great result. I think maybe it's better to report an error when expand macro failing for contract:

@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot 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 Oct 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@chenyukang chenyukang force-pushed the yukang-fix-ice-146808-attr branch from 6a86737 to 6400ed6 Compare October 16, 2025 12:41
@jdonszelmann
Copy link
Contributor

r? jdonszelmann

@rustbot rustbot assigned jdonszelmann and unassigned nnethercote Oct 16, 2025
@jdonszelmann
Copy link
Contributor

could you modify the tests to use #![core::prelude::v1::test]? the same ICE seems to happen on any macro applied to the crate root, and test is stable unlike contracts. Seems to have nothing to do with contract attributes themselves.

@jdonszelmann
Copy link
Contributor

maybe add a comment about that too in the test

@jdonszelmann
Copy link
Contributor

well, actually, with test it seems to be a different ICE? I'm not convinced the root cause of the ICE is the fix you applied 🤔

@jdonszelmann
Copy link
Contributor

jdonszelmann commented Oct 18, 2025

okay, I think I just found a secondary ICE, let me look at that in isolation, though the root cause might be the same as this one

@jdonszelmann
Copy link
Contributor

okay, the other ICE is unrelated and handled here: #147841

@jdonszelmann
Copy link
Contributor

I still think this fixes the wrong thing. Sure, the ICE is gone, but the error points to the wrong location and I'm not even 100% sure why it happens in the first place. Something seems to go fundamentally wrong in which brackets it chooses to match with?

@chenyukang
Copy link
Member Author

chenyukang commented Oct 18, 2025

Something seems to go fundamentally wrong in which brackets it chooses to match with
yes.

seems the span will contain the whole file when parser gets in the the scenario of expecting #![some_attribute(....)], but got the wrong code with #![some_attribute], which is missing the arguments.

I remember for this case the span of here is not right already:

as I wrote in the description, I tried to fix the span, but haven't got a completed fix, I may continue to debug it when get time.

another similar issue is here: #146834

@chenyukang
Copy link
Member Author

it's only happen for #![some_attribute], but not for #[some_attribute]

@jdonszelmann
Copy link
Contributor

Closing, in favor of #147849. See my explanation here: #146808 (comment)

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE all spans must be disjoint (attrs)

4 participants