This commit by itself is supposed to have no effect on behavior. All of
the call sites are updated to preserve their previous behavior.
The behavior changes are in the commits that follow.
For each of these, we need to decide whether they need to be using
`expr_requires_semi_to_be_stmt`, or `expr_requires_comma_to_be_match_arm`,
which are supposed to be 2 different behaviors. Previously they were
conflated into one, causing either too much or too little
parenthesization.
The starting point for this was identical comments on two different
fields, in `ast::VariantData::Struct` and `hir::VariantData::Struct`:
```
// FIXME: investigate making this a `Option<ErrorGuaranteed>`
recovered: bool
```
I tried that, and then found that I needed to add an `ErrorGuaranteed`
to `Recovered::Yes`. Then I ended up using `Recovered` instead of
`Option<ErrorGuaranteed>` for these two places and elsewhere, which
required moving `ErrorGuaranteed` from `rustc_parse` to `rustc_ast`.
This makes things more consistent, because `Recovered` is used in more
places, and there are fewer uses of `bool` and
`Option<ErrorGuaranteed>`. And safer, because it's difficult/impossible
to set `recovered` to `Recovered::Yes` without having emitted an error.
Disallow ambiguous attributes on expressions
This implements the suggestion in [#15701](https://github.com/rust-lang/rust/issues/15701#issuecomment-2033124217) to disallow ambiguous outer attributes on expressions. This should resolve one of the concerns blocking the stabilization of `stmt_expr_attributes`.
The current message for "`->` used for field access" is the following:
```rust
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `->`
--> src/main.rs:2:6
|
2 | a->b;
| ^^ expected one of 8 possible tokens
```
(playground link[1])
This PR tries to address this by adding a dedicated error message and recovery. The proposed error message is:
```
error: `->` used for field access or method call
--> ./tiny_test.rs:2:6
|
2 | a->b;
| ^^ help: try using `.` instead
|
= help: the `.` operator will dereference the value if needed
```
(feel free to bikeshed it as much as necessary)
[1]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7f8b6f4433aa7866124123575456f54e
Signed-off-by: Sasha Pourcelot <sasha.pourcelot@protonmail.com>
For the `MiddleDot` case, current behaviour:
- For a case like `1.2`, `sym1` is `1` and `sym2` is `2`, and `self.token`
holds `1.2`.
- It creates a new ident token from `sym1` that it puts into `self.token`.
- Then it does `bump_with` with a new dot token, which moves the `sym1`
token into `prev_token`.
- Then it does `bump_with` with a new ident token from `sym2`, which moves the
`dot` token into `prev_token` and discards the `sym1` token.
- Then it does `bump`, which puts whatever is next into `self.token`,
moves the `sym2` token into `prev_token`, and discards the `dot` token
altogether.
New behaviour:
- Skips creating and inserting the `sym1` and dot tokens, because they are
unnecessary.
- This also demonstrates that the comment about `Spacing::Alone` is
wrong -- that value is never used. That comment was added in #77250,
and AFAICT it has always been incorrect.
The commit also expands comments. I found this code hard to read
previously, the examples in comments make it easier.
Pass in the span for the field rather than using `prev_token`.
Also rename it `mk_expr_tuple_field_access`, because it doesn't do any
actual parsing, it just creates an expression with what it's given.
Not much of a clarity win by itself, but unlocks additional subsequent
simplifications.
Experimental feature postfix match
This has a basic experimental implementation for the RFC postfix match (rust-lang/rfcs#3295, #121618). [Liaison is](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Postfix.20Match.20Liaison/near/423301844) ```@scottmcm``` with the lang team's [experimental feature gate process](https://github.com/rust-lang/lang-team/blob/master/src/how_to/experiment.md).
This feature has had an RFC for a while, and there has been discussion on it for a while. It would probably be valuable to see it out in the field rather than continue discussing it. This feature also allows to see how popular postfix expressions like this are for the postfix macros RFC, as those will take more time to implement.
It is entirely implemented in the parser, so it should be relatively easy to remove if needed.
This PR is split in to 5 commits to ease review.
1. The implementation of the feature & gating.
2. Add a MatchKind field, fix uses, fix pretty.
3. Basic rustfmt impl, as rustfmt crashes upon seeing this syntax without a fix.
4. Add new MatchSource to HIR for Clippy & other HIR consumers
Interpolated cleanups
Various cleanups I made while working on attempts to remove `Interpolated`, that are worth merging now. Best reviewed one commit at a time.
r? `@petrochenkov`
There are many cases where, due to codegen or a massively unruly codebase,
a deeply nested call(call(call(call(call(call(call(call(call(f())))))))))
can happen. This is a spot where it would be good to grow our stack, so that
we can survive to tell the programmer their code is dubiously written.
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`.
Detect more cases of `=` to `:` typo
When a `Local` is fully parsed, but not followed by a `;`, keep the `:` span arround and mention it. If the type could continue being parsed as an expression, suggest replacing the `:` with a `=`.
```
error: expected one of `!`, `+`, `->`, `::`, `;`, or `=`, found `.`
--> file.rs:2:32
|
2 | let _: std::env::temp_dir().join("foo");
| - ^ expected one of `!`, `+`, `->`, `::`, `;`, or `=`
| |
| while parsing the type for `_`
| help: use `=` if you meant to assign
```
Fix#119665.
When a `Local` is fully parsed, but not followed by a `;`, keep the `:` span
arround and mention it. If the type could continue being parsed as an
expression, suggest replacing the `:` with a `=`.
```
error: expected one of `!`, `+`, `->`, `::`, `;`, or `=`, found `.`
--> file.rs:2:32
|
2 | let _: std::env::temp_dir().join("foo");
| - ^ expected one of `!`, `+`, `->`, `::`, `;`, or `=`
| |
| while parsing the type for `_`
| help: use `=` if you meant to assign
```
Fix#119665.
Stashed errors used to be counted as errors, but could then be
cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed
that, closing the soundness hole. But it introduced other difficulties
because you sometimes have to account for pending stashed errors when
making decisions about whether errors have occured/will occur and it's
easy to overlook these.
This commit aims for a middle ground.
- Stashed errors (not warnings) are counted immediately as emitted
errors, avoiding the possibility of forgetting to consider them.
- The ability to cancel (or downgrade) stashed errors is eliminated, by
disallowing the use of `steal_diagnostic` with errors, and introducing
the more restrictive methods `try_steal_{modify,replace}_and_emit_err`
that can be used instead.
Other things:
- `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both
return `Option<ErrorGuaranteed>`, which enables the removal of two
`delayed_bug` calls and one `Ty::new_error_with_message` call. This is
possible because we store error guarantees in
`DiagCtxt::stashed_diagnostics`.
- Storing the guarantees also saves us having to maintain a counter.
- Calls to the `stashed_err_count` method are no longer necessary
alongside calls to `has_errors`, which is a nice simplification, and
eliminates two more `span_delayed_bug` calls and one FIXME comment.
- Tests are added for three of the four fixed PRs mentioned below.
- `issue-121108.rs`'s output improved slightly, omitting a non-useful
error message.
Fixes#121451.
Fixes#121477.
Fixes#121504.
Fixes#121508.
Add newtypes for bool fields/params/return types
Fixed all the cases of this found with some simple searches for `*/ bool` and `bool /*`; probably many more
There are lots of functions that modify a diagnostic. This can be via a
`&mut Diagnostic` or a `&mut DiagnosticBuilder`, because the latter type
wraps the former and impls `DerefMut`.
This commit converts all the `&mut Diagnostic` occurrences to `&mut
DiagnosticBuilder`. This is a step towards greatly simplifying
`Diagnostic`. Some of the relevant function are made generic, because
they deal with both errors and warnings. No function bodies are changed,
because all the modifier methods are available on both `Diagnostic` and
`DiagnosticBuilder`.
errors: only eagerly translate subdiagnostics
Subdiagnostics don't need to be lazily translated, they can always be eagerly translated. Eager translation is slightly more complex as we need to have a `DiagCtxt` available to perform the translation, which involves slightly more threading of that context.
This slight increase in complexity should enable later simplifications - like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages into the diagnostic structs rather than having them in separate files (working on that was what led to this change).
r? ```@nnethercote```
Subdiagnostics don't need to be lazily translated, they can always be
eagerly translated. Eager translation is slightly more complex as we need
to have a `DiagCtxt` available to perform the translation, which involves
slightly more threading of that context.
This slight increase in complexity should enable later simplifications -
like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages
into the diagnostic structs rather than having them in separate files
(working on that was what led to this change).
Signed-off-by: David Wood <david@davidtw.co>
This mostly works well, and eliminates a couple of delayed bugs.
One annoying thing is that we should really also add an
`ErrorGuaranteed` to `proc_macro::bridge::LitKind::Err`. But that's
difficult because `proc_macro` doesn't have access to `ErrorGuaranteed`,
so we have to fake it.
Be more careful about interpreting a label/lifetime as a mistyped char literal.
Currently the parser interprets any label/lifetime in certain positions as a mistyped char literal, on the assumption that the trailing single quote was accidentally omitted. In such cases it gives an error with a suggestion to add the trailing single quote, and then puts the appropriate char literal into the AST. This behaviour was introduced in #101293.
This is reasonable for a case like this:
```
let c = 'a;
```
because `'a'` is a valid char literal. It's less reasonable for a case like this:
```
let c = 'abc;
```
because `'abc'` is not a valid char literal.
Prior to #120329 this could result in some sub-optimal suggestions in error messages, but nothing else. But #120329 changed `LitKind::from_token_lit` to assume that the char/byte/string literals it receives are valid, and to assert if not. This is reasonable because the lexer does not produce invalid char/byte/string literals in general. But in this "interpret label/lifetime as unclosed char literal" case the parser can produce an invalid char literal with contents such as `abc`, which triggers an assertion failure.
This PR changes the parser so it's more cautious about interpreting labels/lifetimes as unclosed char literals.
Fixes#120397.
r? `@compiler-errors`
Currently the parser will interpret any label/lifetime in certain
positions as a mistyped char literal, on the assumption that the
trailing single quote was accidentally omitted. This is reasonable for a
something like 'a (because 'a' would be valid) but not reasonable for a
something like 'abc (because 'abc' is not valid).
This commit restricts this behaviour only to labels/lifetimes that would
be valid char literals, via the new `could_be_unclosed_char_literal`
function. The commit also augments the `label-is-actually-char.rs` test
in a couple of ways:
- Adds testing of labels/lifetimes with identifiers longer than one
char, e.g. 'abc.
- Adds a new match with simpler patterns, because the
`recover_unclosed_char` call in `parse_pat_with_range_pat` was not
being exercised (in this test or any other ui tests).
Fixes#120397, an assertion failure, which was caused by this behaviour
in the parser interacting with some new stricter char literal checking
added in #120329.
Remove special handling of `box` expressions from parser
#108471 added a temporary hack to parse `box expr`. It's been almost a year since then, so I think it's safe to remove the special handling.
As a drive-by cleanup, move `parser/removed-syntax*` tests to their own directory.
In #119606 I added them and used a `_mv` suffix, but that wasn't great.
A `with_` prefix has three different existing uses.
- Constructors, e.g. `Vec::with_capacity`.
- Wrappers that provide an environment to execute some code, e.g.
`with_session_globals`.
- Consuming chaining methods, e.g. `Span::with_{lo,hi,ctxt}`.
The third case is exactly what we want, so this commit changes
`DiagnosticBuilder::foo_mv` to `DiagnosticBuilder::with_foo`.
Thanks to @compiler-errors for the suggestion.
Instead of taking `seq` as a mutable reference,
`maybe_recover_struct_lit_bad_delims` now consumes `seq` on the recovery
path, and returns `seq` unchanged on the non-recovery path. The commit
also combines an `if` and a `match` to merge two identical paths.
Also change `recover_seq_parse_error` so it receives a `PErr` instead of
a `PResult`, because all the call sites now handle the `Ok`/`Err`
distinction themselves.
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`.
`IntoDiagnostic` defaults to `ErrorGuaranteed`, because errors are the
most common diagnostic level. It makes sense to do likewise for the
closely-related (and much more widely used) `DiagnosticBuilder` type,
letting us write `DiagnosticBuilder<'a, ErrorGuaranteed>` as just
`DiagnosticBuilder<'a>`. This cuts over 200 lines of code due to many
multi-line things becoming single line things.
Add support for `for await` loops
This adds support for `for await` loops. This includes parsing, desugaring in AST->HIR lowering, and adding some support functions to the library.
Given a loop like:
```rust
for await i in iter {
...
}
```
this is desugared to something like:
```rust
let mut iter = iter.into_async_iter();
while let Some(i) = loop {
match core::pin::Pin::new(&mut iter).poll_next(cx) {
Poll::Ready(i) => break i,
Poll::Pending => yield,
}
} {
...
}
```
This PR also adds a basic `IntoAsyncIterator` trait. This is partly for symmetry with the way `Iterator` and `IntoIterator` work. The other reason is that for async iterators it's helpful to have a place apart from the data structure being iterated over to store state. `IntoAsyncIterator` gives us a good place to do this.
I've gated this feature behind `async_for_loop` and opened #118898 as the feature tracking issue.
r? `@compiler-errors`
This commit replaces this pattern:
```
err.into_diagnostic(dcx)
```
with this pattern:
```
dcx.create_err(err)
```
in a lot of places.
It's a little shorter, makes the error level explicit, avoids some
`IntoDiagnostic` imports, and is a necessary prerequisite for the next
commit which will add a `level` arg to `into_diagnostic`.
This requires adding `track_caller` on `create_err` to avoid mucking up
the output of `tests/ui/track-diagnostics/track4.rs`. It probably should
have been there already.
Actually parse async gen blocks correctly
1. I got the control flow in `parse_expr_bottom` messed up, and obviously forgot a test for `async gen`, so we weren't actually ever parsing it correctly.
2. I forgot to gate the span for `async gen {}`, so even if we did parse it, we wouldn't have correctly denied it in `cfg(FALSE)`.
r? eholk
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![];
```
Introduce support for `async gen` blocks
I'm delighted to demonstrate that `async gen` block are not very difficult to support. They're simply coroutines that yield `Poll<Option<T>>` and return `()`.
**This PR is WIP and in draft mode for now** -- I'm mostly putting it up to show folks that it's possible. This PR needs a lang-team experiment associated with it or possible an RFC, since I don't think it falls under the jurisdiction of the `gen` RFC that was recently authored by oli (https://github.com/rust-lang/rfcs/pull/3513, https://github.com/rust-lang/rust/issues/117078).
### Technical note on the pre-generator-transform yield type:
The reason that the underlying coroutines yield `Poll<Option<T>>` and not `Poll<T>` (which would make more sense, IMO, for the pre-transformed coroutine), is because the `TransformVisitor` that is used to turn coroutines into built-in state machine functions would have to destructure and reconstruct the latter into the former, which requires at least inserting a new basic block (for a `switchInt` terminator, to match on the `Poll` discriminant).
This does mean that the desugaring (at the `rustc_ast_lowering` level) of `async gen` blocks is a bit more involved. However, since we already need to intercept both `.await` and `yield` operators, I don't consider it much of a technical burden.
r? `@ghost`
never_patterns: Parse match arms with no body
Never patterns are meant to signal unreachable cases, and thus don't take bodies:
```rust
let ptr: *const Option<!> = ...;
match *ptr {
None => { foo(); }
Some(!),
}
```
This PR makes rustc accept the above, and enforces that an arm has a body xor is a never pattern. This affects parsing of match arms even with the feature off, so this is delicate. (Plus this is my first non-trivial change to the parser).
~~The last commit is optional; it introduces a bit of churn to allow the new suggestions to be machine-applicable. There may be a better solution? I'm not sure.~~ EDIT: I removed that commit
r? `@compiler-errors`
When encountering a bare assignment in a let-chain, suggest turning the
assignment into a `let` expression or an equality check.
```
error: expected expression, found `let` statement
--> $DIR/bad-if-let-suggestion.rs:5:8
|
LL | if let x = 1 && i = 2 {}
| ^^^^^^^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to continue the let-chain
|
LL | if let x = 1 && let i = 2 {}
| +++
help: you might have meant to compare for equality
|
LL | if let x = 1 && i == 2 {}
| +
```
More detail when expecting expression but encountering bad macro argument
On nested macro invocations where the same macro fragment changes fragment type from one to the next, point at the chain of invocations and at the macro fragment definition place, explaining that the change has occurred.
Fix#71039.
```
error: expected expression, found pattern `1 + 1`
--> $DIR/trace_faulty_macros.rs:49:37
|
LL | (let $p:pat = $e:expr) => {test!(($p,$e))};
| ------- -- this is interpreted as expression, but it is expected to be pattern
| |
| this macro fragment matcher is expression
...
LL | (($p:pat, $e:pat)) => {let $p = $e;};
| ------ ^^ expected expression
| |
| this macro fragment matcher is pattern
...
LL | test!(let x = 1+1);
| ------------------
| | |
| | this is expected to be expression
| in this macro invocation
|
= note: when forwarding a matched fragment to another macro-by-example, matchers in the second macro will see an opaque AST of the fragment type, not the underlying tokens
= note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
```
Most notably, this commit changes the `pub use crate::*;` in that file
to `use crate::*;`. This requires a lot of `use` items in other crates
to be adjusted, because everything defined within `rustc_span::*` was
also available via `rustc_span::source_map::*`, which is bizarre.
The commit also removes `SourceMap::span_to_relative_line_string`, which
is unused.
