about summary refs log tree commit diff
path: root/compiler/rustc_parse/src/parser/expr.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-02-25 00:46:04 +0000
committerbors <bors@rust-lang.org>2022-02-25 00:46:04 +0000
commitd4de1f230ca30b7ce08fbf453daebf8b2e7ffcc9 (patch)
tree922a1d8bf2850b2f35dda5bc6cc14e9194c375c1 /compiler/rustc_parse/src/parser/expr.rs
parent4e82f35492ea5c78e19609bf4468f0a686d9a756 (diff)
parentb7e95dee65c35db8f8e07046d445b12d92cbae12 (diff)
downloadrust-d4de1f230ca30b7ce08fbf453daebf8b2e7ffcc9.tar.gz
rust-d4de1f230ca30b7ce08fbf453daebf8b2e7ffcc9.zip
Auto merge of #93368 - eddyb:diagbld-guarantee, r=estebank
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".

That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have:
* `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics
  * can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation
  * asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`)
  * `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called
    (though note that this isn't a real guarantee until after completing the work on
     #69426)
* `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.)
  * can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()`

This PR is a companion to other ongoing work, namely:
* #69426
  and it's ongoing implementation:
  #93222
  the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes
* #93244
  would make the choices of API changes (esp. naming) in this PR fit better overall

In order to be able to let `.emit()` return anything trustable, several changes had to be made:
* `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail)
  * it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()`
* `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic
  * it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user
  * this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it
  * warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early)
  * `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`)
* since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>,
  we have to make `.emit()` idempotent wrt the guarantees it returns
  * thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission
* `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic)
  * the APIs were already mostly identical, allowing for low-effort porting to this new setup
  * only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259)
  * `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective)
  * `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same)

This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each.

r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
Diffstat (limited to 'compiler/rustc_parse/src/parser/expr.rs')
-rw-r--r--compiler/rustc_parse/src/parser/expr.rs40
1 files changed, 23 insertions, 17 deletions
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 7cb8c35b868..c6919779ffd 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -17,7 +17,7 @@ use rustc_ast::{self as ast, AttrStyle, AttrVec, CaptureBy, ExprField, Lit, UnOp
 use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty, TyKind};
 use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
 use rustc_ast_pretty::pprust;
-use rustc_errors::{Applicability, DiagnosticBuilder, PResult};
+use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorReported, PResult};
 use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
 use rustc_session::lint::BuiltinLintDiagnostics;
 use rustc_span::edition::LATEST_STABLE_EDITION;
@@ -684,7 +684,7 @@ impl<'a> Parser<'a> {
         let parser_snapshot_before_type = self.clone();
         let cast_expr = match self.parse_as_cast_ty() {
             Ok(rhs) => mk_expr(self, lhs, rhs),
-            Err(mut type_err) => {
+            Err(type_err) => {
                 // Rewind to before attempting to parse the type with generics, to recover
                 // from situations like `x as usize < y` in which we first tried to parse
                 // `usize < y` as a type with generic arguments.
@@ -717,7 +717,7 @@ impl<'a> Parser<'a> {
                                     .emit();
                                 return Ok(expr);
                             }
-                            Err(mut err) => {
+                            Err(err) => {
                                 err.cancel();
                                 *self = snapshot;
                             }
@@ -773,7 +773,7 @@ impl<'a> Parser<'a> {
 
                         expr
                     }
-                    Err(mut path_err) => {
+                    Err(path_err) => {
                         // Couldn't parse as a path, return original error and parser state.
                         path_err.cancel();
                         *self = parser_snapshot_after_type;
@@ -1127,7 +1127,7 @@ impl<'a> Parser<'a> {
         snapshot: Option<(Self, ExprKind)>,
     ) -> Option<P<Expr>> {
         match (seq.as_mut(), snapshot) {
-            (Err(ref mut err), Some((mut snapshot, ExprKind::Path(None, path)))) => {
+            (Err(err), Some((mut snapshot, ExprKind::Path(None, path)))) => {
                 let name = pprust::path_to_string(&path);
                 snapshot.bump(); // `(`
                 match snapshot.parse_struct_fields(path, false, token::Paren) {
@@ -1138,11 +1138,12 @@ impl<'a> Parser<'a> {
                         let close_paren = self.prev_token.span;
                         let span = lo.to(self.prev_token.span);
                         if !fields.is_empty() {
-                            err.cancel();
-                            let mut err = self.struct_span_err(
+                            let replacement_err = self.struct_span_err(
                                 span,
                                 "invalid `struct` delimiters or `fn` call arguments",
                             );
+                            mem::replace(err, replacement_err).cancel();
+
                             err.multipart_suggestion(
                                 &format!("if `{}` is a struct, use braces as delimiters", name),
                                 vec![
@@ -1166,7 +1167,9 @@ impl<'a> Parser<'a> {
                         return Some(self.mk_expr_err(span));
                     }
                     Ok(_) => {}
-                    Err(mut err) => err.emit(),
+                    Err(mut err) => {
+                        err.emit();
+                    }
                 }
             }
             _ => {}
@@ -1622,9 +1625,11 @@ impl<'a> Parser<'a> {
                 };
                 if let Some(expr) = expr {
                     if matches!(expr.kind, ExprKind::Err) {
-                        self.diagnostic()
-                            .delay_span_bug(self.token.span, &"invalid interpolated expression");
-                        return self.diagnostic().struct_dummy();
+                        let mut err = self
+                            .diagnostic()
+                            .struct_span_err(self.token.span, &"invalid interpolated expression");
+                        err.downgrade_to_delayed_bug();
+                        return err;
                     }
                 }
             }
@@ -1816,6 +1821,7 @@ impl<'a> Parser<'a> {
                 err
             } else {
                 self.struct_span_err(sp, &format!("suffixes on {} are invalid", kind))
+                    .forget_guarantee()
             };
             err.span_label(sp, format!("invalid suffix `{}`", suf));
             err.emit();
@@ -1876,7 +1882,7 @@ impl<'a> Parser<'a> {
                 *self = snapshot;
                 Some(self.mk_expr_err(arr.span))
             }
-            Err(mut e) => {
+            Err(e) => {
                 e.cancel();
                 None
             }
@@ -2097,9 +2103,9 @@ impl<'a> Parser<'a> {
     fn error_missing_if_then_block(
         &self,
         if_span: Span,
-        err: Option<DiagnosticBuilder<'a>>,
+        err: Option<DiagnosticBuilder<'a, ErrorReported>>,
         binop_span: Option<Span>,
-    ) -> DiagnosticBuilder<'a> {
+    ) -> DiagnosticBuilder<'a, ErrorReported> {
         let msg = "this `if` expression has a condition, but no block";
 
         let mut err = if let Some(mut err) = err {
@@ -2379,7 +2385,7 @@ impl<'a> Parser<'a> {
                         return Some(err(self, stmts));
                     }
                 }
-                Err(mut err) => {
+                Err(err) => {
                     err.cancel();
                 }
             }
@@ -2396,7 +2402,7 @@ impl<'a> Parser<'a> {
                 }
                 // We couldn't parse either yet another statement missing it's
                 // enclosing block nor the next arm's pattern or closing brace.
-                Err(mut stmt_err) => {
+                Err(stmt_err) => {
                     stmt_err.cancel();
                     *self = start_snapshot;
                     break;
@@ -2653,7 +2659,7 @@ impl<'a> Parser<'a> {
         let mut base = ast::StructRest::None;
         let mut recover_async = false;
 
-        let mut async_block_err = |e: &mut DiagnosticBuilder<'_>, span: Span| {
+        let mut async_block_err = |e: &mut Diagnostic, span: Span| {
             recover_async = true;
             e.span_label(span, "`async` blocks are only allowed in Rust 2018 or later");
             e.help(&format!("set `edition = \"{}\"` in `Cargo.toml`", LATEST_STABLE_EDITION));