internal: extract shared predicate for AttrId counter sync

[Vastargazing]

Summary

collect_item_tree_attrs() and macro_input_callback() currently need to make identical decisions about which attributes increment item_tree_attr_id, but that logic is split across two places. cfg is still handled separately from is_item_tree_filtered_attr(), so the invariant is easy to desynchronize again.

Background

PR #20892 ("Re-introduce attribute rewrite") introduced macro_input_callback which uses a counter (item_tree_attr_id) to identify which attribute to strip from macro input. This counter must stay synchronized with collect_item_tree_attrs, which assigns AttrIds.

Both functions filter out certain attributes (those matching is_item_tree_filtered_attr()) without incrementing the counter. However, cfg attributes are also filtered — but via a separate special-case, not through is_item_tree_filtered_attr().

In the original code, macro_input_callback handled cfg after the counter-incrementing branch, causing it to count cfg attributes while collect_item_tree_attrs did not. This desynchronized the counter, preventing the attribute macro from being stripped, leading to recursive re-expansion.

Fixed in PR #21205 by reordering the cfg check to come before should_strip_attr().

Reproducer (from PR #21205)

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

Before the fix: the attribute macro was not stripped from the input, causing infinite recursive
re-expansion (S → SSuffix → SSuffixSuffix → …) until the recursion limit was hit and RA hung.

The architectural problem

The comment in attrs.rs states:

"To minimize the risk of bugs, we have one place (here) and one function (is_item_tree_filtered_attr()) that decides whether an attribute participates in name resolution."

But cfg is not in is_item_tree_filtered_attr(). It's handled by a separate special-case in both collect_item_tree_attrs and macro_input_callback. The two functions stay in sync only because both currently duplicate the same filtering decisions manually. That works, but it’s easy to drift again.

Right now both sides effectively implement the same filter table:

• filtered NamedKeyValue → skipped via is_item_tree_filtered_attr()
• cfg token tree → skipped via separate special-case
• filtered TokenTree / Path → skipped via is_item_tree_filtered_attr()
• everything else → increments counter

Proposal

Extract a single predicate that both functions use:

/// Returns `true` if this attribute should be counted for `AttrId` indexing
/// (i.e., it is NOT filtered out by the item tree).
///
/// INVARIANT: This must be used by both `collect_item_tree_attrs` and
/// `macro_input_callback` to keep `item_tree_attr_id` counter in sync.
fn should_count_for_attr_id(meta: &Meta) -> bool {
    match meta {
        Meta::TokenTree { path, .. } if path.is1("cfg") => false,
        Meta::TokenTree { path, .. } | Meta::Path { path } => {
            !(path.segments.len() == 1 && is_item_tree_filtered_attr(path.segments[0].text()))
        }
        Meta::NamedKeyValue { name: Some(n), .. } => !is_item_tree_filtered_attr(n.text()),
        Meta::NamedKeyValue { name: None, .. } => true,
    }
}

This keeps the predicate focused on just one thing: whether the attribute increments item_tree_attr_id.
Cfg evaluation (CfgExpr parsing, cfg_options.check()) remains the caller's responsibility —
it is a separate concern and should not be mixed into the counter-synchronization predicate.

Both collect_item_tree_attrs and macro_input_callback would call should_count_for_attr_id()
instead of duplicating the match arms.

Impact

Mostly just a refactor, but it removes one place where these two paths can drift apart again without anyone noticing.

I have a working implementation ready if this direction looks good.

[ChayimFriedman2]

The existing code seems fine to me, but you can submit your PR and we'll see.