This commit does the following.
- Pulls the code out of `AttrTokenStream::to_token_trees` into a new
function `attrs_and_tokens_to_token_trees`.
- Simplifies `TokenStream::from_ast` by calling the new function. This
is nicer than the old way, which created a temporary
`AttrTokenStream` containing a single `AttrsTarget` (which required
some cloning) just to call `to_token_trees` on it. (It is good to
remove this use of `AttrsTarget` which isn't related to `cfg_attr`
expansion.)
Just some extra sanity checking, making explicit some values not
possible in code working with token trees -- we shouldn't be seeing
explicit delimiter tokens, because they should be represented as
`TokenTree::Delimited`.
expand: fix minor diagnostics bug
The error mentions `///`, when it's actually `//!`:
```
error[E0658]: attributes on expressions are experimental
--> test.rs:4:9
|
4 | //! wah
| ^^^^^^^
|
= note: see issue https://github.com/rust-lang/rust/issues/15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
= help: `///` is for documentation comments. For a plain comment, use `//`.
```
The extra span is now recorded in the new `TokenKind::NtIdent` and
`TokenKind::NtLifetime`. These both consist of a single token, and so
there's no operator precedence problems with inserting them directly
into the token stream.
The other way to do this would be to wrap the ident/lifetime in invisible
delimiters, but there's a lot of code that assumes an interpolated
ident/lifetime fits in a single token, and changing all that code to work with
invisible delimiters would have been a pain. (Maybe it could be done in a
follow-up.)
This change might not seem like much of a win, but it's a first step toward the
much bigger and long-desired removal of `Nonterminal` and
`TokenKind::Interpolated`. That change is big and complex enough that it's
worth doing this piece separately. (Indeed, this commit is based on part of a
late commit in #114647, a prior attempt at that big and complex change.)
The error mentions `///`, when it's actually `//!`:
error[E0658]: attributes on expressions are experimental
--> test.rs:4:9
|
4 | //! wah
| ^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
= help: `///` is for documentation comments. For a plain comment, use `//`.
Currently it only checks calls to functions marked with
`#[rustc_lint_diagnostics]`. This commit changes it to check calls to
any function with an `impl Into<{D,Subd}iagMessage>` parameter. This
greatly improves its coverage and doesn't rely on people remembering to
add `#[rustc_lint_diagnostics]`.
The commit also adds `#[allow(rustc::untranslatable_diagnostic)`]
attributes to places that need it that are caught by the improved lint.
These places that might be easy to convert to translatable diagnostics.
Finally, it also:
- Expands and corrects some comments.
- Does some minor formatting improvements.
- Adds missing `DecorateLint` cases to
`tests/ui-fulldeps/internal-lints/diagnostics.rs`.
Existing names for values of this type are `sess`, `parse_sess`,
`parse_session`, and `ps`. `sess` is particularly annoying because
that's also used for `Session` values, which are often co-located, and
it can be difficult to know which type a value named `sess` refers to.
(That annoyance is the main motivation for this change.) `psess` is nice
and short, which is good for a name used this much.
The commit also renames some `parse_sess_created` values as
`psess_created`.
Currently many diagnostic modifier methods are available on both
`Diagnostic` and `DiagnosticBuilder`. This commit removes most of them
from `Diagnostic`. To minimize the diff size, it keeps them within
`diagnostic.rs` but changes the surrounding `impl Diagnostic` block to
`impl DiagnosticBuilder`. (I intend to move things around later, to give
a more sensible code layout.)
`Diagnostic` keeps a few methods that it still needs, like `sub`,
`arg`, and `replace_args`.
The `forward!` macro, which defined two additional methods per call
(e.g. `note` and `with_note`), is replaced by the `with_fn!` macro,
which defines one additional method per call (e.g. `with_note`). It's
now also only used when necessary -- not all modifier methods currently
need a `with_*` form. (New ones can be easily added as necessary.)
All this also requires changing `trait AddToDiagnostic` so its methods
take `DiagnosticBuilder` instead of `Diagnostic`, which leads to many
mechanical changes. `SubdiagnosticMessageOp` gains a type parameter `G`.
There are three subdiagnostics -- `DelayedAtWithoutNewline`,
`DelayedAtWithNewline`, and `InvalidFlushedDelayedDiagnosticLevel` --
that are created within the diagnostics machinery and appended to
external diagnostics. These are handled at the `Diagnostic` level, which
means it's now hard to construct them via `derive(Diagnostic)`, so
instead we construct them by hand. This has no effect on what they look
like when printed.
There are lots of new `allow` markers for `untranslatable_diagnostics`
and `diagnostics_outside_of_impl`. This is because
`#[rustc_lint_diagnostics]` annotations were present on the `Diagnostic`
modifier methods, but missing from the `DiagnosticBuilder` modifier
methods. They're now present.
This works for most of its call sites. This is nice, because `emit` very
much makes sense as a consuming operation -- indeed,
`DiagnosticBuilderState` exists to ensure no diagnostic is emitted
twice, but it uses runtime checks.
For the small number of call sites where a consuming emit doesn't work,
the commit adds `DiagnosticBuilder::emit_without_consuming`. (This will
be removed in subsequent commits.)
Likewise, `emit_unless` becomes consuming. And `delay_as_bug` becomes
consuming, while `delay_as_bug_without_consuming` is added (which will
also be removed in subsequent commits.)
All this requires significant changes to `DiagnosticBuilder`'s chaining
methods. Currently `DiagnosticBuilder` method chaining uses a
non-consuming `&mut self -> &mut Self` style, which allows chaining to
be used when the chain ends in `emit()`, like so:
```
struct_err(msg).span(span).emit();
```
But it doesn't work when producing a `DiagnosticBuilder` value,
requiring this:
```
let mut err = self.struct_err(msg);
err.span(span);
err
```
This style of chaining won't work with consuming `emit` though. For
that, we need to use to a `self -> Self` style. That also would allow
`DiagnosticBuilder` production to be chained, e.g.:
```
self.struct_err(msg).span(span)
```
However, removing the `&mut self -> &mut Self` style would require that
individual modifications of a `DiagnosticBuilder` go from this:
```
err.span(span);
```
to this:
```
err = err.span(span);
```
There are *many* such places. I have a high tolerance for tedious
refactorings, but even I gave up after a long time trying to convert
them all.
Instead, this commit has it both ways: the existing `&mut self -> Self`
chaining methods are kept, and new `self -> Self` chaining methods are
added, all of which have a `_mv` suffix (short for "move"). Changes to
the existing `forward!` macro lets this happen with very little
additional boilerplate code. I chose to add the suffix to the new
chaining methods rather than the existing ones, because the number of
changes required is much smaller that way.
This doubled chainging is a bit clumsy, but I think it is worthwhile
because it allows a *lot* of good things to subsequently happen. In this
commit, there are many `mut` qualifiers removed in places where
diagnostics are emitted without being modified. In subsequent commits:
- chaining can be used more, making the code more concise;
- more use of chaining also permits the removal of redundant diagnostic
APIs like `struct_err_with_code`, which can be replaced easily with
`struct_err` + `code_mv`;
- `emit_without_diagnostic` can be removed, which simplifies a lot of
machinery, removing the need for `DiagnosticBuilderState`.
Remove edition umbrella features.
In the 2018 edition, there was an "umbrella" feature `#[feature(rust_2018_preview)]` which was used to enable several other features at once. This umbrella mechanism was not used in the 2021 edition and likely will not be used in 2024 either. During 2018 users reported that setting the feature was awkward, especially since they already needed to opt-in via the edition mechanism.
This PR removes this mechanism because I believe it will not be used (and will clean up and simplify the code). I believe that there are better ways to handle features and editions. In short:
- For highly experimental features, that may or may not be involved in an edition, they can implement regular feature gates like `tcx.features().my_feature`.
- For experimental features that *might* be involved in an edition, they should implement gates with `tcx.features().my_feature && span.at_least_rust_20xx()`. This requires the user to still specify `#![feature(my_feature)]`, to avoid disrupting testing of other edition features which are ready and have been accepted within the edition.
- For experimental features that have graduated to definitely be part of an edition, they should implement gates with `tcx.features().my_feature || span.at_least_rust_20xx()`, or just remove the feature check altogether and just check `span.at_least_rust_20xx()`.
- For relatively simple changes, they can skip the whole feature gating thing and just check `span.at_least_rust_20xx()`, and rely on the instability of the edition itself (which requires `-Zunstable-options`) to gate it.
I am working on documenting all of this in the rustc-dev-guide.
This is an extension of the previous commit. It means the output of
something like this:
```
stringify!(let a: Vec<u32> = vec![];)
```
goes from this:
```
let a: Vec<u32> = vec![] ;
```
With this PR, it now produces this string:
```
let a: Vec<u32> = vec![];
```
`tokenstream::Spacing` appears on all `TokenTree::Token` instances,
both punct and non-punct. Its current usage:
- `Joint` means "can join with the next token *and* that token is a
punct".
- `Alone` means "cannot join with the next token *or* can join with the
next token but that token is not a punct".
The fact that `Alone` is used for two different cases is awkward.
This commit augments `tokenstream::Spacing` with a new variant
`JointHidden`, resulting in:
- `Joint` means "can join with the next token *and* that token is a
punct".
- `JointHidden` means "can join with the next token *and* that token is a
not a punct".
- `Alone` means "cannot join with the next token".
This *drastically* improves the output of `print_tts`. For example,
this:
```
stringify!(let a: Vec<u32> = vec![];)
```
currently produces this string:
```
let a : Vec < u32 > = vec! [] ;
```
With this PR, it now produces this string:
```
let a: Vec<u32> = vec![] ;
```
(The space after the `]` is because `TokenTree::Delimited` currently
doesn't have spacing information. The subsequent commit fixes this.)
The new `print_tts` doesn't replicate original code perfectly. E.g.
multiple space characters will be condensed into a single space
character. But it's much improved.
`print_tts` still produces the old, uglier output for code produced by
proc macros. Because we have to translate the generated code from
`proc_macro::Spacing` to the more expressive `token::Spacing`, which
results in too much `proc_macro::Along` usage and no
`proc_macro::JointHidden` usage. So `space_between` still exists and
is used by `print_tts` in conjunction with the `Spacing` field.
This change will also help with the removal of `Token::Interpolated`.
Currently interpolated tokens are pretty-printed nicely via AST pretty
printing. `Token::Interpolated` removal will mean they get printed with
`print_tts`. Without this change, that would result in much uglier
output for code produced by decl macro expansions. With this change, AST
pretty printing and `print_tts` produce similar results.
The commit also tweaks the comments on `proc_macro::Spacing`. In
particular, it refers to "compound tokens" rather than "multi-char
operators" because lifetimes aren't operators.
This keeps track of usage of internal features, and changes the message
to instead tell them that using internal features is not supported.
See MCP 620.
It's a better name, and lets "active features" refer to the features
that are active in a particular program, due to being declared or
enabled by the edition.
The commit also renames `Features::enabled` as `Features::active` to
match this; I changed my mind and have decided that "active" is a little
better thatn "enabled" for this, particularly because a number of
pre-existing comments use "active" in this way.
Finally, the commit renames `Status::Stable` as `Status::Accepted`, to
match `ACCEPTED_FEATURES`.
`State` is used to distinguish active vs accepted vs removed features.
However, these can also be distinguished by their location, in
`ACTIVE_FEATURES`, `ACCEPTED_FEATURES`, and `REMOVED_FEATURES`.
So this commit removes `State` and moves the internals of its variants
next to the `Feature` in each element of `*_FEATURES`, introducing new
types `ActiveFeature` and `RemovedFeature`. (There is no need for
`AcceptedFeature` because `State::Accepted` had no fields.)
This is a tighter type representation, avoids the need for some runtime
checks, and makes the code a bit shorter.
Currently `rust_20XX_preview` features aren't recorded as declared even
when they are explicit declared. Similarly, redundant edition-dependent
features (e.g. `test_2018_feature`) aren't recorded as declared.
This commit marks them as recorded. There is no detectable functional
change, but it makes things more consistent.
The word "active" is currently used in two different and confusing ways:
- `ACTIVE_FEATURES` actually means "available unstable features"
- `Features::active_features` actually means "features declared in the
crate's code", which can include feature within `ACTIVE_FEATURES` but
also others.
(This is also distinct from "enabled" features which includes declared
features but also some edition-specific features automatically enabled
depending on the edition in use.)
This commit changes the `Features::active_features` to
`Features::declared_features` which actually matches its meaning.
Likewise, `Features::active` becomes `Features::declared`.
The new way of doing things:
- Avoids some code duplication.
- Distinguishes the `crate_edition` (which comes from `--edition`) and
the `features_edition` (which combines `--edition` along with any
`rustc_20XX_preview` features), which is useful.
- Has a simpler initial loop, one that just looks for
`rustc_20XX_preview` features in order to compute `features_edition`.
- Creates a fallible alternative to `Features::enabled`, which is
useful.
It's not easy to see how exactly the old and new code are equivalent,
but it's reassuring to know that the test coverage is quite good for
this stuff.
There is a single features (`no_stack_check`) in
`STABLE_REMOVED_FEATURES`. But the treatment of
`STABLE_REMOVED_FEATURES` and `REMOVED_FEATURES` is actually identical.
So this commit just merges them, and uses a comment to record
`no_stack_check`'s unique "stable removed" status.
This also lets `State::Stabilized` (which was a terrible name) be
removed.
This is surprising, but the new comment explains why. It's a logical
conclusion in the drive to avoid `TokenTree` clones.
`TokenTreeCursor` is now only used within `Parser`. It's still needed
due to `replace_prev_and_rewind`.
Previously it removed all other attributes from the crate root.
Now it removes only attributes below itself.
So it becomes possible to configure some global crate properties even for fully unconfigured crates.
`#[cfg]`s are frequently used to gate crate content behind cargo
features. This can lead to very confusing errors when features are
missing. For example, `serde` doesn't have the `derive` feature by
default. Therefore, `serde::Serialize` fails to resolve with a generic
error, even though the macro is present in the docs.
This commit adds a list of all stripped item names to metadata. This is
filled during macro expansion and then, through a fed query, persisted
in metadata. The downstream resolver can then access the metadata to
look at possible candidates for mentioning in the errors.
This slightly increases metadata (800k->809k for the feature-heavy
windows crate), but not enough to really matter.
