diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-08-05 05:40:21 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-08-05 05:40:21 +0200 |
| commit | 20480075bd1706b6a985ac5e6660f9edce6daaea (patch) | |
| tree | e84db528c45353e15f3cf4115e9304a56a73c876 /compiler/rustc_trait_selection/src/traits/select/mod.rs | |
| parent | 3c8b25905ce638ef8144a13ed30c6a6349bb5c06 (diff) | |
| parent | 9d0eaa2ad78ca2995d4f2c4edb61f762ba79846b (diff) | |
| download | rust-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
