about summary refs log tree commit diff
path: root/compiler/rustc_session/src
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_session/src
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_session/src')
-rw-r--r--compiler/rustc_session/src/parse.rs8
-rw-r--r--compiler/rustc_session/src/session.rs90
2 files changed, 51 insertions, 47 deletions
diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs
index 7113f9b0a2f..e287764a52a 100644
--- a/compiler/rustc_session/src/parse.rs
+++ b/compiler/rustc_session/src/parse.rs
@@ -7,7 +7,7 @@ use rustc_ast::node_id::NodeId;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::{Lock, Lrc};
 use rustc_errors::{emitter::SilentEmitter, ColorConfig, Handler};
-use rustc_errors::{error_code, Applicability, DiagnosticBuilder};
+use rustc_errors::{error_code, Applicability, Diagnostic, DiagnosticBuilder, ErrorReported};
 use rustc_feature::{find_feature_issue, GateIssue, UnstableFeatures};
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::ExpnId;
@@ -82,7 +82,7 @@ pub fn feature_err<'a>(
     feature: Symbol,
     span: impl Into<MultiSpan>,
     explain: &str,
-) -> DiagnosticBuilder<'a> {
+) -> DiagnosticBuilder<'a, ErrorReported> {
     feature_err_issue(sess, feature, span, GateIssue::Language, explain)
 }
 
@@ -96,7 +96,7 @@ pub fn feature_err_issue<'a>(
     span: impl Into<MultiSpan>,
     issue: GateIssue,
     explain: &str,
-) -> DiagnosticBuilder<'a> {
+) -> DiagnosticBuilder<'a, ErrorReported> {
     let mut err = sess.span_diagnostic.struct_span_err_with_code(span, explain, error_code!(E0658));
 
     if let Some(n) = find_feature_issue(feature, issue) {
@@ -243,7 +243,7 @@ impl ParseSess {
 
     /// Extend an error with a suggestion to wrap an expression with parentheses to allow the
     /// parser to continue parsing the following operation as part of the same expression.
-    pub fn expr_parentheses_needed(&self, err: &mut DiagnosticBuilder<'_>, span: Span) {
+    pub fn expr_parentheses_needed(&self, err: &mut Diagnostic, span: Span) {
         err.multipart_suggestion(
             "parentheses are required to parse this as an expression",
             vec![(span.shrink_to_lo(), "(".to_string()), (span.shrink_to_hi(), ")".to_string())],
diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs
index 9bcdd7f3da6..b72be735ce4 100644
--- a/compiler/rustc_session/src/session.rs
+++ b/compiler/rustc_session/src/session.rs
@@ -19,7 +19,7 @@ use rustc_errors::annotate_snippet_emitter_writer::AnnotateSnippetEmitterWriter;
 use rustc_errors::emitter::{Emitter, EmitterWriter, HumanReadableErrorType};
 use rustc_errors::json::JsonEmitter;
 use rustc_errors::registry::Registry;
-use rustc_errors::{DiagnosticBuilder, DiagnosticId, ErrorReported};
+use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticId, ErrorReported};
 use rustc_macros::HashStable_Generic;
 pub use rustc_span::def_id::StableCrateId;
 use rustc_span::edition::Edition;
@@ -221,7 +221,7 @@ enum DiagnosticBuilderMethod {
 pub trait SessionDiagnostic<'a> {
     /// Write out as a diagnostic out of `sess`.
     #[must_use]
-    fn into_diagnostic(self, sess: &'a Session) -> DiagnosticBuilder<'a>;
+    fn into_diagnostic(self, sess: &'a Session) -> DiagnosticBuilder<'a, ErrorReported>;
 }
 
 /// Diagnostic message ID, used by `Session.one_time_diagnostics` to avoid
@@ -303,37 +303,39 @@ impl Session {
         self.crate_types.set(crate_types).expect("`crate_types` was initialized twice")
     }
 
-    pub fn struct_span_warn<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> DiagnosticBuilder<'_> {
-        self.diagnostic().struct_span_warn(sp, msg)
-    }
-    pub fn struct_span_force_warn<S: Into<MultiSpan>>(
+    pub fn struct_span_warn<S: Into<MultiSpan>>(
         &self,
         sp: S,
         msg: &str,
-    ) -> DiagnosticBuilder<'_> {
-        self.diagnostic().struct_span_force_warn(sp, msg)
+    ) -> DiagnosticBuilder<'_, ()> {
+        self.diagnostic().struct_span_warn(sp, msg)
     }
     pub fn struct_span_warn_with_code<S: Into<MultiSpan>>(
         &self,
         sp: S,
         msg: &str,
         code: DiagnosticId,
-    ) -> DiagnosticBuilder<'_> {
+    ) -> DiagnosticBuilder<'_, ()> {
         self.diagnostic().struct_span_warn_with_code(sp, msg, code)
     }
-    pub fn struct_warn(&self, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_warn(&self, msg: &str) -> DiagnosticBuilder<'_, ()> {
         self.diagnostic().struct_warn(msg)
     }
-    pub fn struct_force_warn(&self, msg: &str) -> DiagnosticBuilder<'_> {
-        self.diagnostic().struct_force_warn(msg)
-    }
-    pub fn struct_span_allow<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_span_allow<S: Into<MultiSpan>>(
+        &self,
+        sp: S,
+        msg: &str,
+    ) -> DiagnosticBuilder<'_, ()> {
         self.diagnostic().struct_span_allow(sp, msg)
     }
-    pub fn struct_allow(&self, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_allow(&self, msg: &str) -> DiagnosticBuilder<'_, ()> {
         self.diagnostic().struct_allow(msg)
     }
-    pub fn struct_span_err<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_span_err<S: Into<MultiSpan>>(
+        &self,
+        sp: S,
+        msg: &str,
+    ) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_span_err(sp, msg)
     }
     pub fn struct_span_err_with_code<S: Into<MultiSpan>>(
@@ -341,17 +343,25 @@ impl Session {
         sp: S,
         msg: &str,
         code: DiagnosticId,
-    ) -> DiagnosticBuilder<'_> {
+    ) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_span_err_with_code(sp, msg, code)
     }
     // FIXME: This method should be removed (every error should have an associated error code).
-    pub fn struct_err(&self, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_err(&self, msg: &str) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_err(msg)
     }
-    pub fn struct_err_with_code(&self, msg: &str, code: DiagnosticId) -> DiagnosticBuilder<'_> {
+    pub fn struct_err_with_code(
+        &self,
+        msg: &str,
+        code: DiagnosticId,
+    ) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_err_with_code(msg, code)
     }
-    pub fn struct_span_fatal<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_span_fatal<S: Into<MultiSpan>>(
+        &self,
+        sp: S,
+        msg: &str,
+    ) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_span_fatal(sp, msg)
     }
     pub fn struct_span_fatal_with_code<S: Into<MultiSpan>>(
@@ -359,10 +369,10 @@ impl Session {
         sp: S,
         msg: &str,
         code: DiagnosticId,
-    ) -> DiagnosticBuilder<'_> {
+    ) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_span_fatal_with_code(sp, msg, code)
     }
-    pub fn struct_fatal(&self, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_fatal(&self, msg: &str) -> DiagnosticBuilder<'_, ErrorReported> {
         self.diagnostic().struct_fatal(msg)
     }
 
@@ -396,7 +406,7 @@ impl Session {
     pub fn err(&self, msg: &str) {
         self.diagnostic().err(msg)
     }
-    pub fn emit_err<'a>(&'a self, err: impl SessionDiagnostic<'a>) {
+    pub fn emit_err<'a>(&'a self, err: impl SessionDiagnostic<'a>) -> ErrorReported {
         err.into_diagnostic(self).emit()
     }
     #[inline]
@@ -467,7 +477,7 @@ impl Session {
     pub fn span_note_without_error<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
         self.diagnostic().span_note_without_error(sp, msg)
     }
-    pub fn struct_note_without_error(&self, msg: &str) -> DiagnosticBuilder<'_> {
+    pub fn struct_note_without_error(&self, msg: &str) -> DiagnosticBuilder<'_, ()> {
         self.diagnostic().struct_note_without_error(msg)
     }
 
@@ -478,9 +488,9 @@ impl Session {
 
     /// Analogous to calling methods on the given `DiagnosticBuilder`, but
     /// deduplicates on lint ID, span (if any), and message for this `Session`
-    fn diag_once<'a, 'b>(
-        &'a self,
-        diag_builder: &'b mut DiagnosticBuilder<'a>,
+    fn diag_once(
+        &self,
+        diag: &mut Diagnostic,
         method: DiagnosticBuilderMethod,
         msg_id: DiagnosticMessageId,
         message: &str,
@@ -491,39 +501,33 @@ impl Session {
         if fresh {
             match method {
                 DiagnosticBuilderMethod::Note => {
-                    diag_builder.note(message);
+                    diag.note(message);
                 }
                 DiagnosticBuilderMethod::SpanNote => {
                     let span = span_maybe.expect("`span_note` needs a span");
-                    diag_builder.span_note(span, message);
+                    diag.span_note(span, message);
                 }
             }
         }
     }
 
-    pub fn diag_span_note_once<'a, 'b>(
-        &'a self,
-        diag_builder: &'b mut DiagnosticBuilder<'a>,
+    pub fn diag_span_note_once(
+        &self,
+        diag: &mut Diagnostic,
         msg_id: DiagnosticMessageId,
         span: Span,
         message: &str,
     ) {
-        self.diag_once(
-            diag_builder,
-            DiagnosticBuilderMethod::SpanNote,
-            msg_id,
-            message,
-            Some(span),
-        );
+        self.diag_once(diag, DiagnosticBuilderMethod::SpanNote, msg_id, message, Some(span));
     }
 
-    pub fn diag_note_once<'a, 'b>(
-        &'a self,
-        diag_builder: &'b mut DiagnosticBuilder<'a>,
+    pub fn diag_note_once(
+        &self,
+        diag: &mut Diagnostic,
         msg_id: DiagnosticMessageId,
         message: &str,
     ) {
-        self.diag_once(diag_builder, DiagnosticBuilderMethod::Note, msg_id, message, None);
+        self.diag_once(diag, DiagnosticBuilderMethod::Note, msg_id, message, None);
     }
 
     #[inline]