Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not fire unhandled attribute assertion on multi-segment AttributeType::Normal attributes with builtin attribute as first segment #128623

Merged
merged 2 commits into from
Aug 5, 2024

Commits on Aug 4, 2024

  1. Configuration menu
    Copy the full SHA
    9e5c9c1 View commit details
    Browse the repository at this point in the history
  2. check_attr: cover multi-segment attributes on specific check arms

    PR rust-lang#128581 introduced an assertion that all builtin attributes are
    actually checked via `CheckAttrVisitor` and aren't accidentally usable
    on completely unrelated HIR nodes. Unfortunately, the check had
    correctness problems.
    
    The match on attribute path segments looked like
    
    ```rust,ignore
    [sym::should_panic] => /* check is implemented */
    match BUILTIN_ATTRIBUTE_MAP.get(name) {
        // checked below
        Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
        Some(_) => {
            if !name.as_str().starts_with("rustc_") {
                span_bug!(
                    attr.span,
                    "builtin attribute {name:?} not handled by `CheckAttrVisitor`"
                )
            }
        }
        None => (),
    }
    ```
    
    However, it failed to account for edge cases such as an attribute whose:
    
    1. path segments *starts* with a builtin attribute such as
       `should_panic`
    2. which does not start with `rustc_`, and
    3. is also an `AttributeType::Normal` attribute upon registration with
       the builtin attribute map
    
    These conditions when all satisfied cause the span bug to be issued for e.g.
    `#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
    `[sym::should_panic, sym::skip]`).
    
    See <rust-lang#128622>.
    jieyouxu committed Aug 4, 2024
    Configuration menu
    Copy the full SHA
    9d0eaa2 View commit details
    Browse the repository at this point in the history