Skip to content

Conversation

@eihqnh
Copy link
Contributor

@eihqnh eihqnh commented Dec 4, 2025

Description:

This fixes a regression introduced in commit f0e372c ("Rewrite attribute handling"), where cfg attributes
were incorrectly counted during macro input stripping.

Problem

When using attribute macros like maybe-async-cfg, if cfg attributes are present on the item, rust-analyzer may
fail to strip the macro attribute itself from the input. This results in the macro being expanded recursively
(e.g., TestStruct -> TestStructAsync(Correct) -> TestStructAsyncAsync(Wrong)).

522373810-313be124-ff2d-42d1-9dd0-fae782897561 522374103-e62f4dcf-f545-4c56-9ae6-8dceb8efb09e

Cause

The macro_input_callback function in crates/hir-expand/src/cfg_process.rs uses a counter (item_tree_attr_id)
to identify which attribute to strip. This counter was being incremented for cfg attributes.

However, collect_item_tree_attrs (which assigns the attribute IDs) filters out cfg attributes. This caused the
counter in macro_input_callback to desynchronize with the actual attribute IDs, leading to the macro attribute
not being identified and stripped.

Fix

Explicitly skip cfg attributes when determining whether to call should_strip_attr() in macro_input_callback.
This ensures the attribute counter remains in sync with the item tree.
image

Repro

#[maybe_async_cfg::maybe(sync(keep_self), async(feature = "async"))]
#[derive(Debug)]
pub struct TestStruct {
    pub value: u32,
}

#[cfg(feature = "async")]
pub fn test_async_only() {
    let _x = TestStructAsync { value: 42 };
}
#[cfg(not(feature = "async"))]
pub fn test_sync_only() {
    let _y = TestStruct { value: 1 };
}
pub fn test_both_exist() {
    let _a = TestStruct { value: 1 };

    let _b = TestStructAsync { value: 2 };
}
[package]
name = "test-maybe-async"
version = "0.1.0"
edition = "2024"

[dependencies]
maybe-async-cfg = "=0.2.4"

[features]
async = []

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 4, 2025
@eihqnh eihqnh force-pushed the fix/cfg-attr-index-mismatch branch from 326e762 to 7ec2685 Compare December 4, 2025 11:17
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Thanks, that's my mistake.

LGTM, but can you construct a test? (I understand if it's too hard).

@eihqnh eihqnh force-pushed the fix/cfg-attr-index-mismatch branch from 7ec2685 to 80d84e7 Compare December 4, 2025 19:45
@eihqnh
Copy link
Contributor Author

eihqnh commented Dec 4, 2025

Thanks, that's my mistake.

LGTM, but can you construct a test? (I understand if it's too hard).

I added a simple test. Could you give me some suggestions?
cargo test -p hir-def macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg

without this change

cargo test -p hir-def macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg

running 1 test
test macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg ... FAILED

failures:

---- macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg stdout ----


error: expect test failed
   --> crates/hir-def/src/macro_expansion_tests/proc_macros.rs:354:9

You can update all `expect!` tests by running:

    env UPDATE_EXPECT=1 cargo test

To update a single test, place the cursor on `expect` token and use `run` feature of rust-analyzer.

Expect:
----
#[cfg(all())]
#[proc_macros::generate_suffixed_type]
struct S;

struct S;
struct SSuffix;
----

Actual:
----
#[cfg(all())]
#[proc_macros::generate_suffixed_type]
struct S;

----

Diff:
----
#[cfg(all())]
#[proc_macros::generate_suffixed_type]
struct S;

struct S;
struct SSuffix;
----

failures:
    macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 466 filtered out; finished in 0.02s

error: test failed, to rerun pass `-p hir-def --lib`

with this change

test macro_expansion_tests::proc_macros::attribute_macro_stripping_with_cfg ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 466 filtered out; fin

Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Thanks!

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Dec 4, 2025
Merged via the queue into rust-lang:master with commit 98418dc Dec 4, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 4, 2025
@eihqnh eihqnh deleted the fix/cfg-attr-index-mismatch branch December 4, 2025 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants