Implement a lint to warn about unused macro rules
This implements a new lint to warn about unused macro rules (arms/matchers), similar to the `unused_macros` lint added by #41907 that warns about entire macros.
```rust
macro_rules! unused_empty {
(hello) => { println!("Hello, world!") };
() => { println!("empty") }; //~ ERROR: 1st rule of macro `unused_empty` is never used
}
fn main() {
unused_empty!(hello);
}
```
Builds upon #96149 and #96156.
Fixes#73576
Begin fixing all the broken doctests in `compiler/`
Begins to fix#95994.
All of them pass now but 24 of them I've marked with `ignore HELP (<explanation>)` (asking for help) as I'm unsure how to get them to work / if we should leave them as they are.
There are also a few that I marked `ignore` that could maybe be made to work but seem less important.
Each `ignore` has a rough "reason" for ignoring after it parentheses, with
- `(pseudo-rust)` meaning "mostly rust-like but contains foreign syntax"
- `(illustrative)` a somewhat catchall for either a fragment of rust that doesn't stand on its own (like a lone type), or abbreviated rust with ellipses and undeclared types that would get too cluttered if made compile-worthy.
- `(not-rust)` stuff that isn't rust but benefits from the syntax highlighting, like MIR.
- `(internal)` uses `rustc_*` code which would be difficult to make work with the testing setup.
Those reason notes are a bit inconsistently applied and messy though. If that's important I can go through them again and try a more principled approach. When I run `rg '```ignore \(' .` on the repo, there look to be lots of different conventions other people have used for this sort of thing. I could try unifying them all if that would be helpful.
I'm not sure if there was a better existing way to do this but I wrote my own script to help me run all the doctests and wade through the output. If that would be useful to anyone else, I put it here: https://github.com/Elliot-Roberts/rust_doctest_fixing_tool
Add a new Rust attribute to support embedding debugger visualizers
Implemented [this RFC](https://github.com/rust-lang/rfcs/pull/3191) to add support for embedding debugger visualizers into a PDB.
Added a new attribute `#[debugger_visualizer]` and updated the `CrateMetadata` to store debugger visualizers for crate dependencies.
RFC: https://github.com/rust-lang/rfcs/pull/3191
Cleanup `DebuggerVisualizerFile` type and other minor cleanup of queries.
Merge the queries for debugger visualizers into a single query.
Revert move of `resolve_path` to `rustc_builtin_macros`. Update dependencies in Cargo.toml for `rustc_passes`.
Respond to PR comments. Load visualizer files into opaque bytes `Vec<u8>`. Debugger visualizers for dynamically linked crates should not be embedded in the current crate.
Update the unstable book with the new feature. Add the tracking issue for the debugger_visualizer feature.
Respond to PR comments and minor cleanups.
Change `span_suggestion` (and variants) to take `impl ToString` rather
than `String` for the suggested code, as this simplifies the
requirements on the diagnostic derive.
Signed-off-by: David Wood <david.wood@huawei.com>
The code currently ignores the actual delimiter on the RHS and fakes up
a `NoDelim`/`DelimSpan::dummy()` one. This commit changes it to use the
actual delimiter.
The commit also reorders the fields for the `Delimited` variant to match
the `Sequence` variant.
Create (unstable) 2024 edition
[On Zulip](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Deprecating.20macro.20scoping.20shenanigans/near/272860652), there was a small aside regarding creating the 2024 edition now as opposed to later. There was a reasonable amount of support and no stated opposition.
This change creates the 2024 edition in the compiler and creates a prelude for the 2024 edition. There is no current difference between the 2021 and 2024 editions. Cargo and other tools will need to be updated separately, as it's not in the same repository. This change permits the vast majority of work towards the next edition to proceed _now_ instead of waiting until 2024.
For sanity purposes, I've merged the "hello" UI tests into a single file with multiple revisions. Otherwise we'd end up with a file per edition, despite them being essentially identical.
````@rustbot```` label +T-lang +S-waiting-on-review
Not sure on the relevant team, to be honest.
Remove `<mbe::TokenTree as Clone>`
`mbe::TokenTree` doesn't really need to implement `Clone`, and getting rid of that impl leads to some speed-ups.
r? `@petrochenkov`
Loading the fallback bundle in compilation sessions that won't go on to
emit any errors unnecessarily degrades compile time performance, so
lazily create the Fluent bundle when it is first required.
Signed-off-by: David Wood <david.wood@huawei.com>
When a `macro_rules! foo { ... }` invocation is compiled the name used
is `foo`, not `macro_rules!`. This is different to all other macro
invocations, and confused me when I was inserted debugging println
statements for macro evaluation.
This commit changes it to `macro_rules` (or just `macro`), which is what
I expected. There are no externally visible changes.
expand: Remove `ParseSess::missing_fragment_specifiers`
It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff.
cc https://github.com/rust-lang/rust/pull/95747#issuecomment-1091619403
r? ``@nnethercote``
Left overs of #95761
These are just nits. Feel free to close this PR if all modifications are not worth merging.
* `#![feature(decl_macro)]` is not needed anymore in `rustc_expand`
* `tuple_impls` does not require `$Tuple:ident`. I guess it is there to enhance readability?
r? ```@petrochenkov```
refactor: simplify few string related interactions
Few small optimizations:
check_doc_keyword: don't alloc string for emptiness check
check_doc_alias_value: get argument as Symbol to prevent needless string convertions
check_doc_attrs: don't alloc vec, iterate over slice.
replace as_str() check with symbol check
get_single_str_from_tts: don't prealloc string
trivial string to str replace
LifetimeScopeForPath::NonElided use Vec<Symbol> instead of Vec<String>
AssertModuleSource use FxHashSet<Symbol> instead of BTreeSet<String>
CrateInfo.crate_name replace FxHashMap<CrateNum, String> with FxHashMap<CrateNum, Symbol>
It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff.
Remove explicit delimiter token trees from `Delimited`.
They were introduced by the final commit in #95159 and gave a
performance win. But since the introduction of `MatcherLoc` they are no
longer needed. This commit reverts that change, making the code a bit
simpler.
r? `@petrochenkov`
They were introduced by the final commit in #95159 and gave a
performance win. But since the introduction of `MatcherLoc` they are no
longer needed. This commit reverts that change, making the code a bit
simpler.
check_doc_alias_value: get argument as Symbol to prevent needless string convertions
check_doc_attrs: don't alloc vec, iterate over slice. Vec introduced in #83149, but no perf run posted on merge
replace as_str() check with symbol check
get_single_str_from_tts: don't prealloc string
trivial string to str replace
LifetimeScopeForPath::NonElided use Vec<Symbol> instead of Vec<String>
AssertModuleSource use BTreeSet<Symbol> instead of BTreeSet<String>
CrateInfo.crate_name replace FxHashMap<CrateNum, String> with FxHashMap<CrateNum, Symbol>
By heap allocating the argument within `NtPath`, `NtVis`, and `NtStmt`.
This slightly reduces cumulative and peak allocation amounts, most
notably on `deep-vector`.
Currently it's called in `parse_tt` every time a match rule is invoked.
This commit moves it so it's called instead once per match rule, in
`compile_declarative_macro. This is a performance win.
The commit also moves `compute_locs` out of `TtParser`, because there's
no longer any reason for it to be in there.
Use the proc-macro descr to track their individual expansions with
self-profiling events. This will help diagnose performance issues
with slow proc-macros.
In #95555 this was moved out of `parse_tt_inner` and `nameize` into
`compute_locs`. But the next commit will be moving `compute_locs`
outwards to a place that isn't suitable for the missing fragment
identifier checking. So this reinstates the old checking.
Add an option for enabling and disabling Fluent's directionality
isolation markers in output. Disabled by default as these can render in
some terminals and applications.
Signed-off-by: David Wood <david.wood@huawei.com>
Extend loading of Fluent bundles so that bundles can be loaded from the
sysroot based on the language requested by the user, or using a nightly
flag.
Sysroot bundles are loaded from `$sysroot/share/locale/$locale/*.ftl`.
Signed-off-by: David Wood <david.wood@huawei.com>
This commit updates the signatures of all diagnostic functions to accept
types that can be converted into a `DiagnosticMessage`. This enables
existing diagnostic calls to continue to work as before and Fluent
identifiers to be provided. The `SessionDiagnostic` derive just
generates normal diagnostic calls, so these APIs had to be modified to
accept Fluent identifiers.
In addition, loading of the "fallback" Fluent bundle, which contains the
built-in English messages, has been implemented.
Each diagnostic now has "arguments" which correspond to variables in the
Fluent messages (necessary to render a Fluent message) but no API for
adding arguments has been added yet. Therefore, diagnostics (that do not
require interpolation) can be converted to use Fluent identifiers and
will be output as before.
`MultiSpan` contains labels, which are more complicated with the
introduction of diagnostic translation and will use types from
`rustc_errors` - however, `rustc_errors` depends on `rustc_span` so
`rustc_span` cannot use types like `DiagnosticMessage` without
dependency cycles. Introduce a new `rustc_error_messages` crate that can
contain `DiagnosticMessage` and `MultiSpan`.
Signed-off-by: David Wood <david.wood@huawei.com>
Introduce a `DiagnosticMessage` type that will enable diagnostic
messages to be simple strings or Fluent identifiers.
`DiagnosticMessage` is now used in the implementation of the standard
`DiagnosticBuilder` APIs.
Signed-off-by: David Wood <david.wood@huawei.com>
Reduce unnecessary escaping in proc_macro::Literal::character/string
I noticed that https://doc.rust-lang.org/proc_macro/struct.Literal.html#method.character is producing unreadable literals that make macro-expanded code unnecessarily hard to read. Since the proc macro server was using `escape_unicode()`, every char is escaped using `\u{…}` regardless of whether there is any need to do so. For example `Literal::character('=')` would previously produce `'\u{3d}'` which unnecessarily obscures the meaning when reading the macro-expanded code.
I've changed Literal::string also in this PR because `str`'s `Debug` impl is also smarter than just calling `escape_debug` on every char. For example `Literal::string("ferris's")` would previously produce `"ferris\'s"` but will now produce `"ferris's"`.
`parse_tt` currently traverses a `&[TokenTree]` to do matching. But this
is a bad representation for the traversal.
- `TokenTree` is nested, and there's a bunch of expensive and fiddly
state required to handle entering and exiting nested submatchers.
- There are three positions (sequence separators, sequence Kleene ops,
and end of the matcher) that are represented by an index that exceeds
the end of the `&[TokenTree]`, which is clumsy and error-prone.
This commit introduces a new representation called `MatcherLoc` that is
designed specifically for matching. It fixes all the above problems,
making the code much easier to read. A `&[TokenTree]` is converted to a
`&[MatcherLoc]` before matching begins. Despite the cost of the
conversion, it's still a net performance win, because various pieces of
traversal state are computed once up-front, rather than having to be
recomputed repeatedly during the macro matching.
Some improvements worth noting.
- `parse_tt_inner` is *much* easier to read. No more having to compare
`idx` against `len` and read comments to understand what the result
means.
- The handling of `Delimited` in `parse_tt_inner` is now trivial.
- The three end-of-sequence cases in `parse_tt_inner` are now handled in
three separate match arms, and the control flow is much simpler.
- `nameize` is no longer recursive.
- There were two places that issued "missing fragment specifier" errors:
one in `parse_tt_inner()`, and one in `nameize()`. Presumably the
latter was never executed. There's now a single place issuing these
errors, in `compute_locs()`.
- The number of heap allocations done for a `check full` build of
`async-std-1.10.0` (an extreme example of heavy macro use) drops from
11.8M to 2.6M, and most of these occur outside of macro matching.
- The size of `MatcherPos` drops from 64 bytes to 16 bytes. Small enough
that it no longer needs boxing, which partly accounts for the
reduction in allocations.
- The rest of the drop in allocations is due to the removal of
`MatcherKind`, because we no longer need to record anything for the
parent matcher when entering a submatcher.
- Overall it reduces code size by 45 lines.
It's only used in one place, and there we clone and then make a bunch of
modifications. It's clearer if we duplicate more explicitly, and there's
a symmetry now between `sequence()` and `empty_sequence()`.
`parse_tt` needs a way to get from within submatchers make to the
enclosing submatchers. Currently it has two distinct mechanisms for
this:
- `Delimited` submatchers use `MatcherPos::stack` to record stuff about
the parent (and further back ancestors).
- `Sequence` submatchers use `MatcherPosSequence::parent` to point to
the parent matcher position.
Having two mechanisms is really confusing, and it took me a long time to
understand all this.
This commit eliminates `MatcherPos::stack`, and changes `Delimited`
submatchers to use the same mechanism as sequence submatchers. That
mechanism is also changed a bit: instead of storing the entire parent
`MatcherPos`, we now only store the necessary parts from the parent
`MatcherPos`.
Overall this is a small performance win, with the positives outweighing
the negatives, but it's mostly for clarity.
Spellchecking compiler comments
This PR cleans up the rest of the spelling mistakes in the compiler comments. This PR does not change any literal or code spelling issues.
Currently, we detect an exit from a `Delimited` submatcher when `idx`
exceeds the bounds of the current submatcher *and* there is a `stack`
entry.
This commit changes it to something simpler: just look for a
`CloseDelim` token.
Yet more `parse_tt` improvements
Including lots of comment improvements, and an overhaul of how `matches` work that gives big speedups.
r? `@petrochenkov`
Currently, matches within a sequence are recorded in a new empty
`matches` vector. Then when the sequence finishes the matches are merged
into the `matches` vector of the parent.
This commit changes things so that a sequence mp inherits the matches
made so far. This means that additional matches from the sequence don't
need to be merged into the parent. `push_match` becomes more
complicated, and the current sequence depth needs to be tracked. But
it's a sizeable performance win because it avoids one or more
`push_match` calls on every iteration of a sequence.
The commit also removes `match_hi`, which is no longer necessary.
Ignore doc comments in a declarative macro matcher.
Fixes#95267. Reverts to the old behaviour before #95159 introduced a
regression.
r? `@petrochenkov`
Remove `Nonterminal::NtTT`.
It's only needed for macro expansion, not as a general element in the
AST. This commit removes it, adds `NtOrTt` for the parser and macro
expansion cases, and renames the variants in `NamedMatch` to better
match the new type.
r? `@petrochenkov`
It's only needed for macro expansion, not as a general element in the
AST. This commit removes it, adds `NtOrTt` for the parser and macro
expansion cases, and renames the variants in `NamedMatch` to better
match the new type.
Remove `Session::one_time_diagnostic`
This is untracked mutable state, which modified the behaviour of queries.
It was used for 2 things: some full-blown errors, but mostly for lint declaration notes ("the lint level is defined here" notes).
It is replaced by the diagnostic deduplication infra which already exists in the diagnostic emitter.
A new diagnostic level `OnceNote` is introduced specifically for lint notes, to deduplicate subdiagnostics.
As a drive-by, diagnostic emission takes a `&mut` to allow dropping the `SubDiagnostic`s.
Currently it copies a `KleeneOp` and a `Token` out of a
`SequenceRepetition`. It's better to store a reference to the
`SequenceRepetition`, which is now possible due to #95159 having changed
the lifetimes.
The `Lrc` is only relevant within `transcribe()`. There, the `Lrc` is
helpful for the non-`NtTT` cases, because the entire nonterminal is
cloned. But for the `NtTT` cases the inner token tree is cloned (a full
clone) and so the `Lrc` is of no help.
This commit splits the `NtTT` and non-`NtTT` cases, avoiding the useless
`Lrc` in the former case, for the following effect on macro-heavy
crates.
- It reduces the total number of allocations a lot.
- It increases the size of some of the remaining allocations.
- It doesn't affect *peak* memory usage, because the larger allocations
are short-lived.
This overall gives a speed win.
Introduce `TtParser`
These commits make a number of changes to declarative macro expansion, resulting in code that is shorter, simpler, and faster.
Best reviewed one commit at a time.
r? `@petrochenkov`
As its name suggests, `TokenTreeOrTokenTreeSlice` is either a single
`TokenTree` or a slice of them. It has methods `len` and `get_tt` that
let it be treated much like an ordinary slice. The reason it isn't an
ordinary slice is that for `TokenTree::Delimited` the open and close
delimiters are represented implicitly, and when they are needed they are
constructed on the fly with `Delimited::{open,close}_tt`, rather than
being present in memory.
This commit changes `Delimited` so the open and close delimiters are
represented explicitly. As a result, `TokenTreeOrTokenTreeSlice` is no
longer needed and `MatcherPos` and `MatcherTtFrame` can just use an
ordinary slice. `TokenTree::{len,get_tt}` are also removed, because they
were only needed to support `TokenTreeOrTokenTreeSlice`.
The change makes the code shorter and a little bit faster on benchmarks
that use macro expansion heavily, partly because `MatcherPos` is a lot
smaller (less data to `memcpy`) and partly because ordinary slice
operations are faster than `TokenTreeOrTokenTreeSlice::{len,get_tt}`.
By putting them in `TtParser`, we can reuse them for every rule in a
macro. With that done, they can be `SmallVec` instead of `Vec`, and this
is a performance win because these vectors are hot and `SmallVec`
operations are a bit slower due to always needing an "inline or heap?"
check.
This type was a small performance win for `html5ever`, which uses a
macro with hundreds of very simple rules that don't contain any
metavariables. But this type is complicated (extra lifetimes) and
perf-neutral for macros that do have metavariables.
This commit removes `MatcherPosHandle`, simplifying things a lot. This
increases the allocation rate for `html5ever` and similar cases a bit,
but makes things easier for follow-up changes that will improve
performance more than what we lost here.
It currently has no state, just the three methods `parse_tt`,
`parse_tt_inner`, and `bb_items_ambiguity_error`.
This commit is large but trivial, and mostly consists of changes to the
indentation of those methods. Subsequent commits will do more.
There are a few places were we have to construct it, though, and a few
places that are more invasive to change. To do this, we create a
constructor with a long obvious name.
More robust fallback for `use` suggestion
Our old way to suggest where to add `use`s would first look for pre-existing `use`s in the relevant crate/module, and if there are *no* uses, it would fallback on trying to use another item as the basis for the suggestion.
But this was fragile, as illustrated in issue #87613
This PR instead identifies span of the first token after any inner attributes, and uses *that* as the fallback for the `use` suggestion.
Fix#87613
The current structure makes it hard to tell that there are just four
distinct code paths, depending on how many items there are in `bb_items`
and `next_items`. This commit introduces a `match` that clarifies
things.
Ensure stability directives are checked in all cases
Split off #93017
Stability and deprecation were not checked in all cases, for instance if a type error happened.
This PR moves the check earlier in the pipeline to ensure the errors are emitted in all cases.
r? `@lcnr`
Fix invalid lint_node_id being put on a removed stmt
This pull-request remove a invalid `assign_id!` being put on an stmt node.
The problem is that this node is being removed away by a cfg making it unreachable when triggering a buffered lint.
The comment in the other match arm already tell to not assign a id because it could have a `#[cfg()]` so this is just respecting the comment.
Fixes https://github.com/rust-lang/rust/issues/94523
r? ```````@petrochenkov```````
There are three `Option` fields in `MatcherPos` that are only used in
tandem. This commit combines them, making the code slightly easier to
read. (It also makes clear that the `sep` field arguably should have
been `Option<Option<Token>>`!)
To avoid the strange style where comments force `else` onto its own
line.
The commit also removes several else-after-return constructs, which can
be hard to read.
Improve allowness of the unexpected_cfgs lint
This pull-request improve the allowness (`#[allow(...)]`) of the `unexpected_cfgs` lint.
Before this PR only crate level `#![allow(unexpected_cfgs)]` worked, now with this PR it also work when put around `cfg!` or if it is in a upper level. Making it work ~for the attributes `cfg`, `cfg_attr`, ...~ for the same level is awkward as the current code is design to give "Some parent node that is close to this macro call" (cf. https://doc.rust-lang.org/nightly/nightly-rustc/rustc_expand/base/struct.ExpansionData.html) meaning that allow on the same line as an attribute won't work. I'm note even sure if this would be possible.
Found while working on https://github.com/rust-lang/rust/pull/94298.
r? ````````@petrochenkov````````
* Recover from invalid `'label: ` before block.
* Make suggestion to enclose statements in a block multipart.
* Point at `match`, `while`, `loop` and `unsafe` keywords when failing
to parse their expression.
* Do not suggest `{ ; }`.
* Do not suggest `|` when very unlikely to be what was wanted (in `let`
statements).
As an example:
#[test]
#[ignore = "not yet implemented"]
fn test_ignored() {
...
}
Will now render as:
running 2 tests
test tests::test_ignored ... ignored, not yet implemented
test result: ok. 1 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s
