about summary refs log tree commit diff
path: root/compiler/rustc_trait_selection/src/traits/select/mod.rs
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-08-05 05:40:21 +0200
committerGitHub <noreply@github.com>2024-08-05 05:40:21 +0200
commit20480075bd1706b6a985ac5e6660f9edce6daaea (patch)
treee84db528c45353e15f3cf4115e9304a56a73c876 /compiler/rustc_trait_selection/src/traits/select/mod.rs
parent3c8b25905ce638ef8144a13ed30c6a6349bb5c06 (diff)
parent9d0eaa2ad78ca2995d4f2c4edb61f762ba79846b (diff)
downloadrust-20480075bd1706b6a985ac5e6660f9edce6daaea.tar.gz
rust-20480075bd1706b6a985ac5e6660f9edce6daaea.zip
Rollup merge of #128623 - jieyouxu:check-attr-ice, r=nnethercote
Do not fire unhandled attribute assertion on multi-segment `AttributeType::Normal` attributes with builtin attribute as first segment

### The Problem

In #128581 I introduced an assertion to check that all builtin attributes are actually checked via
`CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes.
Unfortunately, the assertion had correctness problems as revealed in #128622.

The match on attribute path segments looked like

```rs,ignore
// Normal handler
[sym::should_panic] => /* check is implemented */
// Fallback handler
[name, ..] => 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 segment matching the name of a builtin attribute such as `should_panic`, and
2. the first segment's symbol does not start with `rustc_`, and
3. the matched builtin attribute is also of `AttributeType::Normal` attribute type 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]`).

### Proposed Solution

This PR tries to remedy that by adjusting all normal/specific handlers to not match exactly on a single segment, but instead match a prefix segment.

i.e.

```rs,ignore
// Normal handler, notice the `, ..` rest pattern
[sym::should_panic, ..] => /* check is implemented */
// Fallback handler
[name, ..] => 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 => (),
}
```

### Review Remarks

This PR contains 2 commits:

1. The first commit adds a regression test. This will ICE without the `CheckAttrVisitor` changes.
2. The second commit adjusts `CheckAttrVisitor` assertion logic. Once this commit is applied, the test should no longer ICE and produce the expected bless stderr.

Fixes #128622.

r? ``@nnethercote`` (since you reviewed #128581)
Diffstat (limited to 'compiler/rustc_trait_selection/src/traits/select/mod.rs')
0 files changed, 0 insertions, 0 deletions