bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Make DiagnosticBuilder::emit consuming.

Browse source 
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`.
This commit is contained in:
Nicholas Nethercote 2024-01-03 12:17:35 +11:00
parent ca2fc426a9
commit b1b9278851
86 changed files with 329 additions and 312 deletions
Download patch file Download diff file Expand all files Collapse all files

35
compiler/rustc_parse/src/parser/item.rs
View file

@ -739,11 +739,14 @@ impl<'a> Parser<'a> {
break;
}
Ok(Some(item)) => items.extend(item),
Err(mut err) => {
Err(err) => {
self.consume_block(Delimiter::Brace, ConsumeClosingDelim::Yes);
err.span_label(open_brace_span, "while parsing this item list starting here")
.span_label(self.prev_token.span, "the item list ends here")
.emit();
err.span_label_mv(
open_brace_span,
"while parsing this item list starting here",
)
.span_label_mv(self.prev_token.span, "the item list ends here")
.emit();
break;
}
}
@ -762,8 +765,8 @@ impl<'a> Parser<'a> {
E0584,
"found a documentation comment that doesn't document anything",
)
.span_label(self.token.span, "this doc comment doesn't document anything")
.help(
.span_label_mv(self.token.span, "this doc comment doesn't document anything")
.help_mv(
"doc comments must come before what they document, if a comment was \
intended use `//`",
)
@ -1106,7 +1109,7 @@ impl<'a> Parser<'a> {
&& self.token.is_keyword(kw::Unsafe)
&& self.look_ahead(1, |t| t.kind == token::OpenDelim(Delimiter::Brace))
{
let mut err = self.expect(&token::OpenDelim(Delimiter::Brace)).unwrap_err();
let err = self.expect(&token::OpenDelim(Delimiter::Brace)).unwrap_err();
err.emit();
unsafety = Unsafe::Yes(self.token.span);
self.eat_keyword(kw::Unsafe);
@ -1198,7 +1201,7 @@ impl<'a> Parser<'a> {
defaultness: Defaultness,
) -> PResult<'a, ItemInfo> {
let impl_span = self.token.span;
let mut err = self.expected_ident_found_err();
let err = self.expected_ident_found_err();
// Only try to recover if this is implementing a trait for a type
let mut impl_info = match self.parse_item_impl(attrs, defaultness) {
@ -1216,7 +1219,7 @@ impl<'a> Parser<'a> {
let before_trait = trai.path.span.shrink_to_lo();
let const_up_to_impl = const_span.with_hi(impl_span.lo());
err.multipart_suggestion(
err.multipart_suggestion_mv(
"you might have meant to write a const trait impl",
vec![(const_up_to_impl, "".to_owned()), (before_trait, "const ".to_owned())],
Applicability::MaybeIncorrect,
@ -1454,8 +1457,8 @@ impl<'a> Parser<'a> {
let ident = this.parse_field_ident("enum", vlo)?;
if this.token == token::Not {
if let Err(mut err) = this.unexpected::<()>() {
err.note(fluent::parse_macro_expands_to_enum_variant).emit();
if let Err(err) = this.unexpected::<()>() {
err.note_mv(fluent::parse_macro_expands_to_enum_variant).emit();
}
this.bump();
@ -1811,7 +1814,7 @@ impl<'a> Parser<'a> {
// `check_trailing_angle_brackets` already emitted a nicer error
// NOTE(eddyb) this was `.cancel()`, but `err`
// gets returned, so we can't fully defuse it.
err.delay_as_bug();
err.delay_as_bug_without_consuming();
}
}
}
@ -1828,7 +1831,7 @@ impl<'a> Parser<'a> {
",",
Applicability::MachineApplicable,
);
err.emit();
err.emit_without_consuming();
recovered = true;
}
@ -1846,7 +1849,7 @@ impl<'a> Parser<'a> {
}
fn expect_field_ty_separator(&mut self) -> PResult<'a, ()> {
if let Err(mut err) = self.expect(&token::Colon) {
if let Err(err) = self.expect(&token::Colon) {
let sm = self.sess.source_map();
let eq_typo = self.token.kind == token::Eq && self.look_ahead(1, |t| t.is_path_start());
let semi_typo = self.token.kind == token::Semi
@ -1862,7 +1865,7 @@ impl<'a> Parser<'a> {
if eq_typo || semi_typo {
self.bump();
// Gracefully handle small typos.
err.span_suggestion_short(
err.span_suggestion_short_mv(
self.prev_token.span,
"field names and their types are separated with `:`",
":",
@ -2598,7 +2601,7 @@ impl<'a> Parser<'a> {
let (mut params, _) = self.parse_paren_comma_seq(|p| {
p.recover_diff_marker();
let snapshot = p.create_snapshot_for_diagnostic();
let param = p.parse_param_general(req_name, first_param).or_else(|mut e| {
let param = p.parse_param_general(req_name, first_param).or_else(|e| {
e.emit();
let lo = p.prev_token.span;
p.restore_snapshot(snapshot);