Reduce kw::Empty usage, part 4 #138347 #139039
Conversation
It makes it clearer that the symbol is unused and doesn't matter.
To use one `kw::Empty` instead of two. It's a little more direct this way, and avoids `kw::Empty` being used for both "no string" and "empty string".
This is part of the implementation of `#[doc(keyword = "match")]` attributes used by `std` to provide documentation for keywords. `is_doc_keyword` currently does a crude keyword range test that's intended to catch all keywords but misses `kw::Yeet`. This commit changes it to use `Symbol` methods, including the new `is_weak` method (required for `union`). `Symbol` methods are much less prone to falling out of date if new keywords are added.
By changing two of the fields to use `Option<Ident>` instead of `Ident`. As a result, `None` now means "no identifier", which is much clearer than using an empty identifier.
Using in-band values for exceptional cases is error prone. This commit changes it to an `Option`.
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
rustbot
added
A-attributes
nnethercote
changed the title
| @@ -79,10 +79,11 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> { | |||
| if apa.counter <= 1 || !apa.has_expensive_expr_after_last_attr { | |||
| continue; | |||
| } | |||
| let first_bind_ident = apa.first_bind_ident.unwrap(); | |||
There was a problem hiding this comment.
.unwrap() use is a little bit dangerous
| let first_bind_ident = apa.first_bind_ident.unwrap(); | |
| debug_assert!(apa.first_bind_ident.is_some()); | |
| let Some(first_bind_ident) = apa.first_bind_ident else { return; }; |
There was a problem hiding this comment.
debug_asserts are only for performance-sensitive checks, if something unexpected happens here it's better to have a panic always (unwrap) than only under some build configurations (debug assert).
| } | ||
|
|
||
| fn is_decode_placeholder(&self) -> bool { | ||
| self.dollar_crate_name == kw::Empty |
There was a problem hiding this comment.
This is not a placeholder for dollar_crate_name, it means that the whole SyntaxContextData is a placeholder and will never be used (there's a bunch of debug_asserts in this file checking for it).
I think it's ok to use sym::dummy here, but there's an ongoing work (#129827) to eliminate these placeholders entirely, so it's better not to change this otherwise.
petrochenkov
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #129827) made this pull request unmergeable. Please resolve the merge conflicts. |
Another step towards #137978.
r? @petrochenkov