From ae8b3e8fc6e95a0f68fc49338f394d670e233883 Mon Sep 17 00:00:00 2001
From: Mazdak Farrokhzad <twingoow@gmail.com>
Date: Mon, 23 Sep 2019 04:45:21 +0200
Subject: [PATCH 1/4] Introduce a diagnostic stashing API.
---
src/librustc/session/mod.rs | 1 +
src/librustc_driver/lib.rs | 3 +-
src/librustc_errors/diagnostic_builder.rs | 41 +++++++----
src/librustc_errors/lib.rs | 84 +++++++++++++++++++----
4 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
index a24fed8f21c5a..49342d95fdb03 100644
--- a/src/librustc/session/mod.rs
+++ b/src/librustc/session/mod.rs
@@ -321,6 +321,7 @@ impl Session {
}
pub fn compile_status(&self) -> Result<(), ErrorReported> {
if self.has_errors() {
+ self.diagnostic().emit_stashed_diagnostics();
Err(ErrorReported)
} else {
Ok(())
diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs
index f99e65b4494a7..4a8681367410e 100644
--- a/src/librustc_driver/lib.rs
+++ b/src/librustc_driver/lib.rs
@@ -296,7 +296,6 @@ pub fn run_compiler(
);
Ok(())
})?;
- return sess.compile_status();
} else {
let mut krate = compiler.parse()?.take();
pretty::visit_crate(sess, &mut krate, ppm);
@@ -307,8 +306,8 @@ pub fn run_compiler(
ppm,
compiler.output_file().as_ref().map(|p| &**p),
);
- return sess.compile_status();
}
+ return sess.compile_status();
}
if callbacks.after_parsing(compiler) == Compilation::Stop {
diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs
index e85388bfea29c..cc60bf89c7eca 100644
--- a/src/librustc_errors/diagnostic_builder.rs
+++ b/src/librustc_errors/diagnostic_builder.rs
@@ -1,10 +1,6 @@
-use crate::Diagnostic;
-use crate::DiagnosticId;
-use crate::DiagnosticStyledString;
-use crate::Applicability;
+use crate::{Diagnostic, DiagnosticId, DiagnosticStyledString};
+use crate::{Applicability, Level, Handler, StashKey};
-use crate::Level;
-use crate::Handler;
use std::fmt::{self, Debug};
use std::ops::{Deref, DerefMut};
use std::thread::panicking;
@@ -117,18 +113,30 @@ impl<'a> DiagnosticBuilder<'a> {
}
}
- /// Buffers the diagnostic for later emission, unless handler
- /// has disabled such buffering.
- pub fn buffer(mut self, buffered_diagnostics: &mut Vec<Diagnostic>) {
+ /// Stashes diagnostic for possible later improvement in a different,
+ /// later stage of the compiler. The diagnostic can be accessed with
+ /// the provided `span` and `key` through `.steal_diagnostic` on `Handler`.
+ ///
+ /// As with `buffer`, this is unless the handler has disabled such buffering.
+ pub fn stash(self, span: Span, key: StashKey) {
+ if let Some((diag, handler)) = self.into_diagnostic() {
+ handler.stash_diagnostic(span, key, diag);
+ }
+ }
+
+ /// Converts the builder to a `Diagnostic` for later emission,
+ /// unless handler has disabled such buffering.
+ pub fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a Handler)> {
if self.0.handler.flags.dont_buffer_diagnostics ||
self.0.handler.flags.treat_err_as_bug.is_some()
{
self.emit();
- return;
+ return None;
}
- // We need to use `ptr::read` because `DiagnosticBuilder`
- // implements `Drop`.
+ let handler = self.0.handler;
+
+ // We need to use `ptr::read` because `DiagnosticBuilder` implements `Drop`.
let diagnostic;
unsafe {
diagnostic = std::ptr::read(&self.0.diagnostic);
@@ -137,7 +145,14 @@ impl<'a> DiagnosticBuilder<'a> {
// Logging here is useful to help track down where in logs an error was
// actually emitted.
debug!("buffer: diagnostic={:?}", diagnostic);
- buffered_diagnostics.push(diagnostic);
+
+ Some((diagnostic, handler))
+ }
+
+ /// Buffers the diagnostic for later emission,
+ /// unless handler has disabled such buffering.
+ pub fn buffer(self, buffered_diagnostics: &mut Vec<Diagnostic>) {
+ buffered_diagnostics.extend(self.into_diagnostic().map(|(diag, _)| diag));
}
/// Convenience function for internal use, clients should use one of the
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
index 1fe5b71d7b1cf..40f63ae1eee33 100644
--- a/src/librustc_errors/lib.rs
+++ b/src/librustc_errors/lib.rs
@@ -17,7 +17,7 @@ use emitter::{Emitter, EmitterWriter};
use registry::Registry;
use rustc_data_structures::sync::{self, Lrc, Lock};
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::stable_hasher::StableHasher;
use std::borrow::Cow;
@@ -326,6 +326,18 @@ struct HandlerInner {
/// this handler. These hashes is used to avoid emitting the same error
/// twice.
emitted_diagnostics: FxHashSet<u128>,
+
+ /// Stashed diagnostics emitted in one stage of the compiler that may be
+ /// stolen by other stages (e.g. to improve them and add more information).
+ /// The stashed diagnostics count towards the total error count.
+ /// When `.abort_if_errors()` is called, these are also emitted.
+ stashed_diagnostics: FxIndexMap<(Span, StashKey), Diagnostic>,
+}
+
+/// A key denoting where from a diagnostic was stashed.
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
+pub enum StashKey {
+ ItemNoType,
}
fn default_track_diagnostic(_: &Diagnostic) {}
@@ -354,7 +366,9 @@ pub struct HandlerFlags {
impl Drop for HandlerInner {
fn drop(&mut self) {
- if self.err_count == 0 {
+ self.emit_stashed_diagnostics();
+
+ if !self.has_errors() {
let bugs = std::mem::replace(&mut self.delayed_span_bugs, Vec::new());
let has_bugs = !bugs.is_empty();
for bug in bugs {
@@ -419,6 +433,7 @@ impl Handler {
taught_diagnostics: Default::default(),
emitted_diagnostic_codes: Default::default(),
emitted_diagnostics: Default::default(),
+ stashed_diagnostics: Default::default(),
}),
}
}
@@ -445,6 +460,31 @@ impl Handler {
inner.emitted_diagnostics = Default::default();
inner.deduplicated_err_count = 0;
inner.err_count = 0;
+ inner.stashed_diagnostics.clear();
+ }
+
+ /// Stash a given diagnostic with the given `Span` and `StashKey` as the key for later stealing.
+ /// If the diagnostic with this `(span, key)` already exists, this will result in an ICE.
+ pub fn stash_diagnostic(&self, span: Span, key: StashKey, diag: Diagnostic) {
+ if let Some(old) = self.inner.borrow_mut().stashed_diagnostics.insert((span, key), diag) {
+ // We are removing a previously stashed diagnostic which should not happen.
+ // Create a builder and drop it on the floor to get an ICE.
+ drop(DiagnosticBuilder::new_diagnostic(self, old));
+ }
+ }
+
+ /// Steal a previously stashed diagnostic with the given `Span` and `StashKey` as the key.
+ pub fn steal_diagnostic(&self, span: Span, key: StashKey) -> Option<DiagnosticBuilder<'_>> {
+ self.inner
+ .borrow_mut()
+ .stashed_diagnostics
+ .remove(&(span, key))
+ .map(|diag| DiagnosticBuilder::new_diagnostic(self, diag))
+ }
+
+ /// Emit all stashed diagnostics.
+ pub fn emit_stashed_diagnostics(&self) {
+ self.inner.borrow_mut().emit_stashed_diagnostics();
}
pub fn struct_dummy(&self) -> DiagnosticBuilder<'_> {
@@ -617,11 +657,11 @@ impl Handler {
}
pub fn err_count(&self) -> usize {
- self.inner.borrow().err_count
+ self.inner.borrow().err_count()
}
pub fn has_errors(&self) -> bool {
- self.err_count() > 0
+ self.inner.borrow().has_errors()
}
pub fn print_error_count(&self, registry: &Registry) {
@@ -629,11 +669,11 @@ impl Handler {
}
pub fn abort_if_errors(&self) {
- self.inner.borrow().abort_if_errors()
+ self.inner.borrow_mut().abort_if_errors()
}
pub fn abort_if_errors_and_should_abort(&self) {
- self.inner.borrow().abort_if_errors_and_should_abort()
+ self.inner.borrow_mut().abort_if_errors_and_should_abort()
}
pub fn must_teach(&self, code: &DiagnosticId) -> bool {
@@ -671,6 +711,12 @@ impl HandlerInner {
self.emitter.emit_diagnostic(&db);
}
+ /// Emit all stashed diagnostics.
+ fn emit_stashed_diagnostics(&mut self) {
+ let diags = self.stashed_diagnostics.drain(..).map(|x| x.1).collect::<Vec<_>>();
+ diags.iter().for_each(|diag| self.emit_diagnostic(diag));
+ }
+
fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
if diagnostic.cancelled() {
return;
@@ -713,10 +759,12 @@ impl HandlerInner {
}
fn treat_err_as_bug(&self) -> bool {
- self.flags.treat_err_as_bug.map(|c| self.err_count >= c).unwrap_or(false)
+ self.flags.treat_err_as_bug.map(|c| self.err_count() >= c).unwrap_or(false)
}
fn print_error_count(&mut self, registry: &Registry) {
+ self.emit_stashed_diagnostics();
+
let s = match self.deduplicated_err_count {
0 => return,
1 => "aborting due to previous error".to_string(),
@@ -760,14 +808,26 @@ impl HandlerInner {
}
}
- fn abort_if_errors_and_should_abort(&self) {
- if self.err_count > 0 && !self.continue_after_error {
+ fn err_count(&self) -> usize {
+ self.err_count + self.stashed_diagnostics.len()
+ }
+
+ fn has_errors(&self) -> bool {
+ self.err_count() > 0
+ }
+
+ fn abort_if_errors_and_should_abort(&mut self) {
+ self.emit_stashed_diagnostics();
+
+ if self.has_errors() && !self.continue_after_error {
FatalError.raise();
}
}
- fn abort_if_errors(&self) {
- if self.err_count > 0 {
+ fn abort_if_errors(&mut self) {
+ self.emit_stashed_diagnostics();
+
+ if self.has_errors() {
FatalError.raise();
}
}
@@ -826,7 +886,7 @@ impl HandlerInner {
fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() {
- let s = match (self.err_count, self.flags.treat_err_as_bug.unwrap_or(0)) {
+ let s = match (self.err_count(), self.flags.treat_err_as_bug.unwrap_or(0)) {
(0, _) => return,
(1, 1) => "aborting due to `-Z treat-err-as-bug=1`".to_string(),
(1, _) => return,
From 62d85849d0a9a828dc58a1820469daf80a2b5b52 Mon Sep 17 00:00:00 2001
From: Mazdak Farrokhzad <twingoow@gmail.com>
Date: Mon, 23 Sep 2019 04:48:22 +0200
Subject: [PATCH 2/4] Add parser recovery for `const $ident = $expr;`. Then use
the diagnostics-stealing API to stash parser errors and enrich them with type
information in typeck.
---
src/librustc_typeck/collect.rs | 49 ++++++++++++++------
src/libsyntax/parse/parser/item.rs | 47 +++++++++++++++++--
src/test/ui/suggestions/const-no-type.rs | 46 ++++++++++++++++++
src/test/ui/suggestions/const-no-type.stderr | 38 +++++++++++++++
4 files changed, 164 insertions(+), 16 deletions(-)
create mode 100644 src/test/ui/suggestions/const-no-type.rs
create mode 100644 src/test/ui/suggestions/const-no-type.stderr
diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs
index d2e9203779cc8..e6e0cb88fbd23 100644
--- a/src/librustc_typeck/collect.rs
+++ b/src/librustc_typeck/collect.rs
@@ -46,7 +46,7 @@ use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc::hir::GenericParamKind;
use rustc::hir::{self, CodegenFnAttrFlags, CodegenFnAttrs, Unsafety};
-use errors::{Applicability, DiagnosticId};
+use errors::{Applicability, DiagnosticId, StashKey};
struct OnlySelfBounds(bool);
@@ -1149,18 +1149,41 @@ fn infer_placeholder_type(
def_id: DefId,
body_id: hir::BodyId,
span: Span,
+ item_ident: Ident,
) -> Ty<'_> {
let ty = tcx.typeck_tables_of(def_id).node_type(body_id.hir_id);
- let mut diag = bad_placeholder_type(tcx, span);
- if ty != tcx.types.err {
- diag.span_suggestion(
- span,
- "replace `_` with the correct type",
- ty.to_string(),
- Applicability::MaybeIncorrect,
- );
+
+ // If this came from a free `const` or `static mut?` item,
+ // then the user may have written e.g. `const A = 42;`.
+ // In this case, the parser has stashed a diagnostic for
+ // us to improve in typeck so we do that now.
+ match tcx.sess.diagnostic().steal_diagnostic(span, StashKey::ItemNoType) {
+ Some(mut err) => {
+ // The parser provided a sub-optimal `HasPlaceholders` suggestion for the type.
+ // We are typeck and have the real type, so remove that and suggest the actual type.
+ err.suggestions.clear();
+ err.span_suggestion(
+ span,
+ "provide a type for the item",
+ format!("{}: {}", item_ident, ty),
+ Applicability::MachineApplicable,
+ )
+ .emit();
+ }
+ None => {
+ let mut diag = bad_placeholder_type(tcx, span);
+ if ty != tcx.types.err {
+ diag.span_suggestion(
+ span,
+ "replace `_` with the correct type",
+ ty.to_string(),
+ Applicability::MaybeIncorrect,
+ );
+ }
+ diag.emit();
+ }
}
- diag.emit();
+
ty
}
@@ -1192,7 +1215,7 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option<Ty<
TraitItemKind::Const(ref ty, body_id) => {
body_id.and_then(|body_id| {
if let hir::TyKind::Infer = ty.node {
- Some(infer_placeholder_type(tcx, def_id, body_id, ty.span))
+ Some(infer_placeholder_type(tcx, def_id, body_id, ty.span, item.ident))
} else {
None
}
@@ -1214,7 +1237,7 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option<Ty<
}
ImplItemKind::Const(ref ty, body_id) => {
if let hir::TyKind::Infer = ty.node {
- infer_placeholder_type(tcx, def_id, body_id, ty.span)
+ infer_placeholder_type(tcx, def_id, body_id, ty.span, item.ident)
} else {
icx.to_ty(ty)
}
@@ -1246,7 +1269,7 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option<Ty<
ItemKind::Static(ref ty, .., body_id)
| ItemKind::Const(ref ty, body_id) => {
if let hir::TyKind::Infer = ty.node {
- infer_placeholder_type(tcx, def_id, body_id, ty.span)
+ infer_placeholder_type(tcx, def_id, body_id, ty.span, item.ident)
} else {
icx.to_ty(ty)
}
diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs
index cf196645e4f7b..0d073f0cc97b1 100644
--- a/src/libsyntax/parse/parser/item.rs
+++ b/src/libsyntax/parse/parser/item.rs
@@ -24,7 +24,7 @@ use crate::symbol::{kw, sym};
use std::mem;
use log::debug;
use rustc_target::spec::abi::Abi;
-use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
+use errors::{Applicability, DiagnosticBuilder, DiagnosticId, StashKey};
/// Whether the type alias or associated type is a concrete type or an opaque type.
#[derive(Debug)]
@@ -1477,10 +1477,23 @@ impl<'a> Parser<'a> {
}
}
+ /// Parse `["const" | ("static" "mut"?)] $ident ":" $ty = $expr` with
+ /// `["const" | ("static" "mut"?)]` already parsed and stored in `m`.
+ ///
+ /// When `m` is `"const"`, `$ident` may also be `"_"`.
fn parse_item_const(&mut self, m: Option<Mutability>) -> PResult<'a, ItemInfo> {
let id = if m.is_none() { self.parse_ident_or_underscore() } else { self.parse_ident() }?;
- self.expect(&token::Colon)?;
- let ty = self.parse_ty()?;
+
+ // Parse the type of a `const` or `static mut?` item.
+ // That is, the `":" $ty` fragment.
+ let ty = if self.token == token::Eq {
+ self.recover_missing_const_type(id, m)
+ } else {
+ // Not `=` so expect `":"" $ty` as usual.
+ self.expect(&token::Colon)?;
+ self.parse_ty()?
+ };
+
self.expect(&token::Eq)?;
let e = self.parse_expr()?;
self.expect(&token::Semi)?;
@@ -1491,6 +1504,34 @@ impl<'a> Parser<'a> {
Ok((id, item, None))
}
+ /// We were supposed to parse `:` but instead, we're already at `=`.
+ /// This means that the type is missing.
+ fn recover_missing_const_type(&mut self, id: Ident, m: Option<Mutability>) -> P<Ty> {
+ // Construct the error and stash it away with the hope
+ // that typeck will later enrich the error with a type.
+ let kind = match m {
+ Some(Mutability::Mutable) => "static mut",
+ Some(Mutability::Immutable) => "static",
+ None => "const",
+ };
+ let mut err = self.struct_span_err(id.span, &format!("missing type for `{}` item", kind));
+ err.span_suggestion(
+ id.span,
+ "provide a type for the item",
+ format!("{}: <type>", id),
+ Applicability::HasPlaceholders,
+ );
+ err.stash(id.span, StashKey::ItemNoType);
+
+ // The user intended that the type be inferred,
+ // so treat this as if the user wrote e.g. `const A: _ = expr;`.
+ P(Ty {
+ node: TyKind::Infer,
+ span: id.span,
+ id: ast::DUMMY_NODE_ID,
+ })
+ }
+
/// Parses `type Foo = Bar;` or returns `None`
/// without modifying the parser state.
fn eat_type(&mut self) -> Option<PResult<'a, (Ident, AliasKind, Generics)>> {
diff --git a/src/test/ui/suggestions/const-no-type.rs b/src/test/ui/suggestions/const-no-type.rs
new file mode 100644
index 0000000000000..99200a965dd21
--- /dev/null
+++ b/src/test/ui/suggestions/const-no-type.rs
@@ -0,0 +1,46 @@
+// In the cases below, the type is missing from the `const` and `static` items.
+//
+// Here, we test that we:
+//
+// a) Perform parser recovery.
+//
+// b) Emit a diagnostic with the actual inferred type to RHS of `=` as the suggestion.
+
+fn main() {}
+
+// These will not reach typeck:
+
+#[cfg(FALSE)]
+const C2 = 42;
+//~^ ERROR missing type for `const` item
+//~| HELP provide a type for the item
+//~| SUGGESTION C2: <type>
+
+#[cfg(FALSE)]
+static S2 = "abc";
+//~^ ERROR missing type for `static` item
+//~| HELP provide a type for the item
+//~| SUGGESTION S2: <type>
+
+#[cfg(FALSE)]
+static mut SM2 = "abc";
+//~^ ERROR missing type for `static mut` item
+//~| HELP provide a type for the item
+//~| SUGGESTION SM2: <type>
+
+// These will, so the diagnostics should be stolen by typeck:
+
+const C = 42;
+//~^ ERROR missing type for `const` item
+//~| HELP provide a type for the item
+//~| SUGGESTION C: i32
+
+static S = Vec::<String>::new();
+//~^ ERROR missing type for `static` item
+//~| HELP provide a type for the item
+//~| SUGGESTION S: std::vec::Vec<std::string::String>
+
+static mut SM = "abc";
+//~^ ERROR missing type for `static mut` item
+//~| HELP provide a type for the item
+//~| SUGGESTION &'static str
diff --git a/src/test/ui/suggestions/const-no-type.stderr b/src/test/ui/suggestions/const-no-type.stderr
new file mode 100644
index 0000000000000..c4f17109dc5c7
--- /dev/null
+++ b/src/test/ui/suggestions/const-no-type.stderr
@@ -0,0 +1,38 @@
+error: missing type for `const` item
+ --> $DIR/const-no-type.rs:33:7
+ |
+LL | const C = 42;
+ | ^ help: provide a type for the item: `C: i32`
+
+error: missing type for `static` item
+ --> $DIR/const-no-type.rs:38:8
+ |
+LL | static S = Vec::<String>::new();
+ | ^ help: provide a type for the item: `S: std::vec::Vec<std::string::String>`
+
+error: missing type for `static mut` item
+ --> $DIR/const-no-type.rs:43:12
+ |
+LL | static mut SM = "abc";
+ | ^^ help: provide a type for the item: `SM: &'static str`
+
+error: missing type for `const` item
+ --> $DIR/const-no-type.rs:14:7
+ |
+LL | const C2 = 42;
+ | ^^ help: provide a type for the item: `C2: <type>`
+
+error: missing type for `static` item
+ --> $DIR/const-no-type.rs:20:8
+ |
+LL | static S2 = "abc";
+ | ^^ help: provide a type for the item: `S2: <type>`
+
+error: missing type for `static mut` item
+ --> $DIR/const-no-type.rs:26:12
+ |
+LL | static mut SM2 = "abc";
+ | ^^^ help: provide a type for the item: `SM2: <type>`
+
+error: aborting due to 6 previous errors
+
From 62fc4d36dfeedbf0795f36a8d08c39e0f4e41632 Mon Sep 17 00:00:00 2001
From: Mazdak Farrokhzad <twingoow@gmail.com>
Date: Mon, 23 Sep 2019 19:29:02 +0200
Subject: [PATCH 3/4] stash_diagnostic: ICE in a different way
---
src/librustc_errors/lib.rs | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
index 40f63ae1eee33..cdaa528e8e497 100644
--- a/src/librustc_errors/lib.rs
+++ b/src/librustc_errors/lib.rs
@@ -466,10 +466,15 @@ impl Handler {
/// Stash a given diagnostic with the given `Span` and `StashKey` as the key for later stealing.
/// If the diagnostic with this `(span, key)` already exists, this will result in an ICE.
pub fn stash_diagnostic(&self, span: Span, key: StashKey, diag: Diagnostic) {
- if let Some(old) = self.inner.borrow_mut().stashed_diagnostics.insert((span, key), diag) {
+ let mut inner = self.inner.borrow_mut();
+ if let Some(mut old_diag) = inner.stashed_diagnostics.insert((span, key), diag) {
// We are removing a previously stashed diagnostic which should not happen.
- // Create a builder and drop it on the floor to get an ICE.
- drop(DiagnosticBuilder::new_diagnostic(self, old));
+ old_diag.level = Bug;
+ old_diag.note(&format!(
+ "{}:{}: already existing stashed diagnostic with (span = {:?}, key = {:?})",
+ file!(), line!(), span, key
+ ));
+ inner.emit_explicit_bug(&old_diag);
}
}
@@ -676,6 +681,11 @@ impl Handler {
self.inner.borrow_mut().abort_if_errors_and_should_abort()
}
+ /// `true` if we haven't taught a diagnostic with this code already.
+ /// The caller must then teach the user about such a diagnostic.
+ ///
+ /// Used to suppress emitting the same error multiple times with extended explanation when
+ /// calling `-Zteach`.
pub fn must_teach(&self, code: &DiagnosticId) -> bool {
self.inner.borrow_mut().must_teach(code)
}
@@ -698,11 +708,6 @@ impl Handler {
}
impl HandlerInner {
- /// `true` if we haven't taught a diagnostic with this code already.
- /// The caller must then teach the user about such a diagnostic.
- ///
- /// Used to suppress emitting the same error multiple times with extended explanation when
- /// calling `-Zteach`.
fn must_teach(&mut self, code: &DiagnosticId) -> bool {
self.taught_diagnostics.insert(code.clone())
}
@@ -833,7 +838,11 @@ impl HandlerInner {
}
fn span_bug<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> ! {
- self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(sp));
+ self.emit_explicit_bug(Diagnostic::new(Bug, msg).set_span(sp));
+ }
+
+ fn emit_explicit_bug(&mut self, diag: &Diagnostic) -> ! {
+ self.emit_diagnostic(diag);
self.abort_if_errors_and_should_abort();
panic!(ExplicitBug);
}
From f70665a84692a80a820fccdaed19df5dde94c533 Mon Sep 17 00:00:00 2001
From: Mazdak Farrokhzad <twingoow@gmail.com>
Date: Mon, 23 Sep 2019 22:28:14 +0200
Subject: [PATCH 4/4] cleanup librustc_errors Handler code.
---
src/librustc_errors/lib.rs | 301 +++++++++++++++------------
src/librustc_typeck/check/wfcheck.rs | 2 +-
src/libsyntax/ext/base.rs | 4 -
src/libsyntax/ext/expand.rs | 2 +-
src/libsyntax_ext/format.rs | 4 +-
5 files changed, 170 insertions(+), 143 deletions(-)
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
index cdaa528e8e497..c01dcd94c725e 100644
--- a/src/librustc_errors/lib.rs
+++ b/src/librustc_errors/lib.rs
@@ -302,6 +302,9 @@ pub struct Handler {
inner: Lock<HandlerInner>,
}
+/// This inner struct exists to keep it all behind a single lock;
+/// this is done to prevent possible deadlocks in a multi-threaded compiler,
+/// as well as inconsistent state observation.
struct HandlerInner {
flags: HandlerFlags,
/// The number of errors that have been emitted, including duplicates.
@@ -382,52 +385,65 @@ impl Drop for HandlerInner {
}
impl Handler {
- pub fn with_tty_emitter(color_config: ColorConfig,
- can_emit_warnings: bool,
- treat_err_as_bug: Option<usize>,
- cm: Option<Lrc<SourceMapperDyn>>)
- -> Handler {
- Handler::with_tty_emitter_and_flags(
+ pub fn with_tty_emitter(
+ color_config: ColorConfig,
+ can_emit_warnings: bool,
+ treat_err_as_bug: Option<usize>,
+ cm: Option<Lrc<SourceMapperDyn>>,
+ ) -> Self {
+ Self::with_tty_emitter_and_flags(
color_config,
cm,
HandlerFlags {
can_emit_warnings,
treat_err_as_bug,
.. Default::default()
- })
+ },
+ )
}
- pub fn with_tty_emitter_and_flags(color_config: ColorConfig,
- cm: Option<Lrc<SourceMapperDyn>>,
- flags: HandlerFlags)
- -> Handler {
+ pub fn with_tty_emitter_and_flags(
+ color_config: ColorConfig,
+ cm: Option<Lrc<SourceMapperDyn>>,
+ flags: HandlerFlags,
+ ) -> Self {
let emitter = Box::new(EmitterWriter::stderr(
- color_config, cm, false, false, None, flags.external_macro_backtrace));
- Handler::with_emitter_and_flags(emitter, flags)
- }
-
- pub fn with_emitter(can_emit_warnings: bool,
- treat_err_as_bug: Option<usize>,
- e: Box<dyn Emitter + sync::Send>)
- -> Handler {
+ color_config,
+ cm,
+ false,
+ false,
+ None,
+ flags.external_macro_backtrace,
+ ));
+ Self::with_emitter_and_flags(emitter, flags)
+ }
+
+ pub fn with_emitter(
+ can_emit_warnings: bool,
+ treat_err_as_bug: Option<usize>,
+ emitter: Box<dyn Emitter + sync::Send>,
+ ) -> Self {
Handler::with_emitter_and_flags(
- e,
+ emitter,
HandlerFlags {
can_emit_warnings,
treat_err_as_bug,
.. Default::default()
- })
+ },
+ )
}
- pub fn with_emitter_and_flags(e: Box<dyn Emitter + sync::Send>, flags: HandlerFlags) -> Handler
- {
- Handler {
+ pub fn with_emitter_and_flags(
+ emitter: Box<dyn Emitter + sync::Send>,
+ flags: HandlerFlags
+ ) -> Self {
+ Self {
flags,
inner: Lock::new(HandlerInner {
flags,
err_count: 0,
deduplicated_err_count: 0,
- emitter: e,
+ emitter,
continue_after_error: true,
delayed_span_bugs: Vec::new(),
taught_diagnostics: Default::default(),
@@ -474,7 +490,8 @@ impl Handler {
"{}:{}: already existing stashed diagnostic with (span = {:?}, key = {:?})",
file!(), line!(), span, key
));
- inner.emit_explicit_bug(&old_diag);
+ inner.emit_diag_at_span(old_diag, span);
+ panic!(ExplicitBug);
}
}
@@ -492,34 +509,35 @@ impl Handler {
self.inner.borrow_mut().emit_stashed_diagnostics();
}
+ /// Construct a dummy builder with `Level::Cancelled`.
+ ///
+ /// Using this will neither report anything to the user (e.g. a warning),
+ /// nor will compilation cancel as a result.
pub fn struct_dummy(&self) -> DiagnosticBuilder<'_> {
DiagnosticBuilder::new(self, Level::Cancelled, "")
}
- pub fn struct_span_warn<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Warning, msg);
- result.set_span(sp);
- if !self.flags.can_emit_warnings {
- result.cancel();
- }
+ /// Construct a builder at the `Warning` level at the given `span` and with the `msg`.
+ pub fn struct_span_warn(&self, span: impl Into<MultiSpan>, msg: &str) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_warn(msg);
+ result.set_span(span);
result
}
- pub fn struct_span_warn_with_code<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str,
- code: DiagnosticId)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Warning, msg);
- result.set_span(sp);
+
+ /// Construct a builder at the `Warning` level at the given `span` and with the `msg`.
+ /// Also include a code.
+ pub fn struct_span_warn_with_code(
+ &self,
+ span: impl Into<MultiSpan>,
+ msg: &str,
+ code: DiagnosticId,
+ ) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_span_warn(span, msg);
result.code(code);
- if !self.flags.can_emit_warnings {
- result.cancel();
- }
result
}
+
+ /// Construct a builder at the `Warning` level with the `msg`.
pub fn struct_warn(&self, msg: &str) -> DiagnosticBuilder<'_> {
let mut result = DiagnosticBuilder::new(self, Level::Warning, msg);
if !self.flags.can_emit_warnings {
@@ -527,136 +545,141 @@ impl Handler {
}
result
}
- pub fn struct_span_err<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
- result.set_span(sp);
+
+ /// Construct a builder at the `Error` level at the given `span` and with the `msg`.
+ pub fn struct_span_err(&self, span: impl Into<MultiSpan>, msg: &str) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_err(msg);
+ result.set_span(span);
result
}
- pub fn struct_span_err_with_code<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str,
- code: DiagnosticId)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
- result.set_span(sp);
+
+ /// Construct a builder at the `Error` level at the given `span`, with the `msg`, and `code`.
+ pub fn struct_span_err_with_code(
+ &self,
+ span: impl Into<MultiSpan>,
+ msg: &str,
+ code: DiagnosticId,
+ ) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_span_err(span, msg);
result.code(code);
result
}
+
+ /// Construct a builder at the `Error` level with the `msg`.
// FIXME: This method should be removed (every error should have an associated error code).
pub fn struct_err(&self, msg: &str) -> DiagnosticBuilder<'_> {
DiagnosticBuilder::new(self, Level::Error, msg)
}
- pub fn struct_err_with_code(
- &self,
- msg: &str,
- code: DiagnosticId,
- ) -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
+
+ /// Construct a builder at the `Error` level with the `msg` and the `code`.
+ pub fn struct_err_with_code(&self, msg: &str, code: DiagnosticId) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_err(msg);
result.code(code);
result
}
- pub fn struct_span_fatal<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Fatal, msg);
- result.set_span(sp);
+
+ /// Construct a builder at the `Fatal` level at the given `span` and with the `msg`.
+ pub fn struct_span_fatal(
+ &self,
+ span: impl Into<MultiSpan>,
+ msg: &str,
+ ) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_fatal(msg);
+ result.set_span(span);
result
}
- pub fn struct_span_fatal_with_code<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str,
- code: DiagnosticId)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Fatal, msg);
- result.set_span(sp);
+
+ /// Construct a builder at the `Fatal` level at the given `span`, with the `msg`, and `code`.
+ pub fn struct_span_fatal_with_code(
+ &self,
+ span: impl Into<MultiSpan>,
+ msg: &str,
+ code: DiagnosticId,
+ ) -> DiagnosticBuilder<'_> {
+ let mut result = self.struct_span_fatal(span, msg);
result.code(code);
result
}
+
+ /// Construct a builder at the `Error` level with the `msg`.
pub fn struct_fatal(&self, msg: &str) -> DiagnosticBuilder<'_> {
DiagnosticBuilder::new(self, Level::Fatal, msg)
}
- pub fn span_fatal<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> FatalError {
- self.emit_diagnostic(Diagnostic::new(Fatal, msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+ pub fn span_fatal(&self, span: impl Into<MultiSpan>, msg: &str) -> FatalError {
+ self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span);
FatalError
}
- pub fn span_fatal_with_code<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str,
- code: DiagnosticId)
- -> FatalError {
- self.emit_diagnostic(Diagnostic::new_with_code(Fatal, Some(code), msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_fatal_with_code(
+ &self,
+ span: impl Into<MultiSpan>,
+ msg: &str,
+ code: DiagnosticId,
+ ) -> FatalError {
+ self.emit_diag_at_span(Diagnostic::new_with_code(Fatal, Some(code), msg), span);
FatalError
}
- pub fn span_err<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
- self.emit_diagnostic(Diagnostic::new(Error, msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
- }
- pub fn mut_span_err<S: Into<MultiSpan>>(&self,
- sp: S,
- msg: &str)
- -> DiagnosticBuilder<'_> {
- let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
- result.set_span(sp);
- result
+
+ pub fn span_err(&self, span: impl Into<MultiSpan>, msg: &str) {
+ self.emit_diag_at_span(Diagnostic::new(Error, msg), span);
}
- pub fn span_err_with_code<S: Into<MultiSpan>>(&self, sp: S, msg: &str, code: DiagnosticId) {
- self.emit_diagnostic(Diagnostic::new_with_code(Error, Some(code), msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_err_with_code(&self, span: impl Into<MultiSpan>, msg: &str, code: DiagnosticId) {
+ self.emit_diag_at_span(Diagnostic::new_with_code(Error, Some(code), msg), span);
}
- pub fn span_warn<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
- self.emit_diagnostic(Diagnostic::new(Warning, msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_warn(&self, span: impl Into<MultiSpan>, msg: &str) {
+ self.emit_diag_at_span(Diagnostic::new(Warning, msg), span);
}
- pub fn span_warn_with_code<S: Into<MultiSpan>>(&self, sp: S, msg: &str, code: DiagnosticId) {
- self.emit_diagnostic(Diagnostic::new_with_code(Warning, Some(code), msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_warn_with_code(&self, span: impl Into<MultiSpan>, msg: &str, code: DiagnosticId) {
+ self.emit_diag_at_span(Diagnostic::new_with_code(Warning, Some(code), msg), span);
}
- pub fn span_bug<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> ! {
- self.inner.borrow_mut().span_bug(sp, msg)
+
+ pub fn span_bug(&self, span: impl Into<MultiSpan>, msg: &str) -> ! {
+ self.inner.borrow_mut().span_bug(span, msg)
}
- pub fn delay_span_bug<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
- self.inner.borrow_mut().delay_span_bug(sp, msg)
+
+ pub fn delay_span_bug(&self, span: impl Into<MultiSpan>, msg: &str) {
+ self.inner.borrow_mut().delay_span_bug(span, msg)
}
- pub fn span_bug_no_panic<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
- self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_bug_no_panic(&self, span: impl Into<MultiSpan>, msg: &str) {
+ self.emit_diag_at_span(Diagnostic::new(Bug, msg), span);
}
- pub fn span_note_without_error<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
- self.emit_diagnostic(Diagnostic::new(Note, msg).set_span(sp));
- self.abort_if_errors_and_should_abort();
+
+ pub fn span_note_without_error(&self, span: impl Into<MultiSpan>, msg: &str) {
+ self.emit_diag_at_span(Diagnostic::new(Note, msg), span);
}
- pub fn span_note_diag(&self,
- sp: Span,
- msg: &str)
- -> DiagnosticBuilder<'_> {
+
+ pub fn span_note_diag(&self, span: Span, msg: &str) -> DiagnosticBuilder<'_> {
let mut db = DiagnosticBuilder::new(self, Note, msg);
- db.set_span(sp);
+ db.set_span(span);
db
}
+
pub fn failure(&self, msg: &str) {
self.inner.borrow_mut().failure(msg);
}
+
pub fn fatal(&self, msg: &str) -> FatalError {
self.inner.borrow_mut().fatal(msg)
}
+
pub fn err(&self, msg: &str) {
self.inner.borrow_mut().err(msg);
}
+
pub fn warn(&self, msg: &str) {
let mut db = DiagnosticBuilder::new(self, Warning, msg);
db.emit();
}
+
pub fn note_without_error(&self, msg: &str) {
- let mut db = DiagnosticBuilder::new(self, Note, msg);
- db.emit();
+ DiagnosticBuilder::new(self, Note, msg).emit();
}
+
pub fn bug(&self, msg: &str) -> ! {
self.inner.borrow_mut().bug(msg)
}
@@ -698,6 +721,12 @@ impl Handler {
self.inner.borrow_mut().emit_diagnostic(diagnostic)
}
+ fn emit_diag_at_span(&self, mut diag: Diagnostic, sp: impl Into<MultiSpan>) {
+ let mut inner = self.inner.borrow_mut();
+ inner.emit_diagnostic(diag.set_span(sp));
+ inner.abort_if_errors_and_should_abort();
+ }
+
pub fn emit_artifact_notification(&self, path: &Path, artifact_type: &str) {
self.inner.borrow_mut().emit_artifact_notification(path, artifact_type)
}
@@ -837,17 +866,17 @@ impl HandlerInner {
}
}
- fn span_bug<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) -> ! {
- self.emit_explicit_bug(Diagnostic::new(Bug, msg).set_span(sp));
+ fn span_bug(&mut self, sp: impl Into<MultiSpan>, msg: &str) -> ! {
+ self.emit_diag_at_span(Diagnostic::new(Bug, msg), sp);
+ panic!(ExplicitBug);
}
- fn emit_explicit_bug(&mut self, diag: &Diagnostic) -> ! {
- self.emit_diagnostic(diag);
+ fn emit_diag_at_span(&mut self, mut diag: Diagnostic, sp: impl Into<MultiSpan>) {
+ self.emit_diagnostic(diag.set_span(sp));
self.abort_if_errors_and_should_abort();
- panic!(ExplicitBug);
}
- fn delay_span_bug<S: Into<MultiSpan>>(&mut self, sp: S, msg: &str) {
+ fn delay_span_bug(&mut self, sp: impl Into<MultiSpan>, msg: &str) {
if self.treat_err_as_bug() {
// FIXME: don't abort here if report_delayed_bugs is off
self.span_bug(sp, msg);
@@ -862,18 +891,20 @@ impl HandlerInner {
}
fn fatal(&mut self, msg: &str) -> FatalError {
- if self.treat_err_as_bug() {
- self.bug(msg);
- }
- self.emit_diagnostic(&Diagnostic::new(Fatal, msg));
+ self.emit_error(Fatal, msg);
FatalError
}
fn err(&mut self, msg: &str) {
+ self.emit_error(Error, msg);
+ }
+
+ /// Emit an error; level should be `Error` or `Fatal`.
+ fn emit_error(&mut self, level: Level, msg: &str,) {
if self.treat_err_as_bug() {
self.bug(msg);
}
- self.emit_diagnostic(&Diagnostic::new(Error, msg));
+ self.emit_diagnostic(&Diagnostic::new(level, msg));
}
fn bug(&mut self, msg: &str) -> ! {
diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs
index b0e886a2aa2eb..f7e766bb84d57 100644
--- a/src/librustc_typeck/check/wfcheck.rs
+++ b/src/librustc_typeck/check/wfcheck.rs
@@ -832,7 +832,7 @@ fn check_method_receiver<'fcx, 'tcx>(
}
fn e0307(fcx: &FnCtxt<'fcx, 'tcx>, span: Span, receiver_ty: Ty<'_>) {
- fcx.tcx.sess.diagnostic().mut_span_err(
+ fcx.tcx.sess.diagnostic().struct_span_err(
span,
&format!("invalid `self` parameter type: {:?}", receiver_ty)
).note("type of `self` must be `Self` or a type that dereferences to it")
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index a6be5b101788e..36d066b026933 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -1041,10 +1041,6 @@ impl<'a> ExtCtxt<'a> {
pub fn span_err_with_code<S: Into<MultiSpan>>(&self, sp: S, msg: &str, code: DiagnosticId) {
self.parse_sess.span_diagnostic.span_err_with_code(sp, msg, code);
}
- pub fn mut_span_err<S: Into<MultiSpan>>(&self, sp: S, msg: &str)
- -> DiagnosticBuilder<'a> {
- self.parse_sess.span_diagnostic.mut_span_err(sp, msg)
- }
pub fn span_warn<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
self.parse_sess.span_diagnostic.span_warn(sp, msg);
}
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index b80c530731dfc..c3b1f91d5317d 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -384,7 +384,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let attr = attr::find_by_name(item.attrs(), sym::derive)
.expect("`derive` attribute should exist");
let span = attr.span;
- let mut err = self.cx.mut_span_err(span,
+ let mut err = self.cx.struct_span_err(span,
"`derive` may only be applied to structs, enums and unions");
if let ast::AttrStyle::Inner = attr.style {
let trait_list = derives.iter()
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index 26455df17b896..2765346b333cf 100644
--- a/src/libsyntax_ext/format.rs
+++ b/src/libsyntax_ext/format.rs
@@ -295,7 +295,7 @@ impl<'a, 'b> Context<'a, 'b> {
.filter(|fmt| fmt.precision_span.is_some())
.count();
if self.names.is_empty() && !numbered_position_args && count != self.args.len() {
- e = self.ecx.mut_span_err(
+ e = self.ecx.struct_span_err(
sp,
&format!(
"{} positional argument{} in format string, but {}",
@@ -336,7 +336,7 @@ impl<'a, 'b> Context<'a, 'b> {
sp = MultiSpan::from_span(self.fmtsp);
}
- e = self.ecx.mut_span_err(sp,
+ e = self.ecx.struct_span_err(sp,
&format!("invalid reference to positional {} ({})",
arg_list,
self.describe_num_args()));