Rollup merge of #66427 - Mark-Simulacrum:errors-json, r=Centril

Move the JSON error emitter to librustc_errors

This is done both as a cleanup (it makes little sense for this emitter to be in libsyntax), but also as part of broader work to decouple Session from librustc itself.

Along the way, this also moves SourceMap to syntax_pos, which is also nice for the above reasons, as well as allowing dropping the SourceMapper trait from code. This had the unfortunate side-effect of moving `FatalError` to rustc_data_structures (it's needed in syntax_pos, due to SourceMap, but putting it there feels somehow worse).

Cargo.lock

@@ -4439,6 +4439,7 @@ version = "0.0.0"
 dependencies = [
  "arena",
  "cfg-if",
+ "log",
  "rustc_data_structures",
  "rustc_index",
  "rustc_macros",

src/librustc/session/mod.rs

@@ -22,7 +22,7 @@ use errors::emitter::HumanReadableErrorType;
 use errors::annotate_snippet_emitter_writer::{AnnotateSnippetEmitterWriter};
 use syntax::edition::Edition;
 use syntax::feature_gate::{self, AttributeType};
-use syntax::json::JsonEmitter;
+use errors::json::JsonEmitter;
 use syntax::source_map;
 use syntax::sess::{ParseSess, ProcessCfgMod};
 use syntax::symbol::Symbol;

src/librustc_codegen_ssa/back/write.rs

@@ -23,7 +23,8 @@ use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_fs_util::link_or_copy;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::Lrc;
-use rustc_errors::{Handler, Level, FatalError, DiagnosticId, SourceMapperDyn};
+use rustc_errors::{Handler, Level, FatalError, DiagnosticId};
+use syntax_pos::source_map::SourceMap;
 use rustc_errors::emitter::{Emitter};
 use rustc_target::spec::MergeFunctions;
 use syntax::attr;
@@ -1679,7 +1680,7 @@ impl Emitter for SharedEmitter {
         }
         drop(self.sender.send(SharedEmitterMessage::AbortIfErrors));
     }
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>> {
+    fn source_map(&self) -> Option<&Lrc<SourceMap>> {
         None
     }
 }

src/librustc_errors/annotate_snippet_emitter_writer.rs

@@ -6,9 +6,10 @@
 //! [annotate_snippets]: https://docs.rs/crate/annotate-snippets/
 
 use syntax_pos::{SourceFile, MultiSpan, Loc};
+use syntax_pos::source_map::SourceMap;
 use crate::{
     Level, CodeSuggestion, Diagnostic, Emitter,
-    SourceMapperDyn, SubDiagnostic, DiagnosticId
+    SubDiagnostic, DiagnosticId
 };
 use crate::emitter::FileWithAnnotatedLines;
 use rustc_data_structures::sync::Lrc;
@@ -20,7 +21,7 @@ use annotate_snippets::formatter::DisplayListFormatter;
 
 /// Generates diagnostics using annotate-snippet
 pub struct AnnotateSnippetEmitterWriter {
-    source_map: Option<Lrc<SourceMapperDyn>>,
+    source_map: Option<Lrc<SourceMap>>,
     /// If true, hides the longer explanation text
     short_message: bool,
     /// If true, will normalize line numbers with `LL` to prevent noise in UI test diffs.
@@ -49,7 +50,7 @@ impl Emitter for AnnotateSnippetEmitterWriter {
                                    &suggestions);
     }
 
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>> {
+    fn source_map(&self) -> Option<&Lrc<SourceMap>> {
         self.source_map.as_ref()
     }
 
@@ -61,7 +62,7 @@ impl Emitter for AnnotateSnippetEmitterWriter {
 /// Collects all the data needed to generate the data structures needed for the
 /// `annotate-snippets` library.
 struct DiagnosticConverter<'a> {
-    source_map: Option<Lrc<SourceMapperDyn>>,
+    source_map: Option<Lrc<SourceMap>>,
     level: Level,
     message: String,
     code: Option<DiagnosticId>,
@@ -168,7 +169,7 @@ impl<'a>  DiagnosticConverter<'a> {
 
 impl AnnotateSnippetEmitterWriter {
     pub fn new(
-        source_map: Option<Lrc<SourceMapperDyn>>,
+        source_map: Option<Lrc<SourceMap>>,
         short_message: bool,
         external_macro_backtrace: bool,
     ) -> Self {

src/librustc_errors/emitter.rs

@@ -10,10 +10,11 @@
 use Destination::*;
 
 use syntax_pos::{SourceFile, Span, MultiSpan};
+use syntax_pos::source_map::SourceMap;
 
 use crate::{
     Level, CodeSuggestion, Diagnostic, SubDiagnostic, pluralize,
-    SuggestionStyle, SourceMapper, SourceMapperDyn, DiagnosticId,
+    SuggestionStyle, DiagnosticId,
 };
 use crate::Level::Error;
 use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
@@ -49,7 +50,7 @@ impl HumanReadableErrorType {
     pub fn new_emitter(
         self,
         dst: Box<dyn Write + Send>,
-        source_map: Option<Lrc<SourceMapperDyn>>,
+        source_map: Option<Lrc<SourceMap>>,
         teach: bool,
         terminal_width: Option<usize>,
         external_macro_backtrace: bool,
@@ -192,7 +193,7 @@ pub trait Emitter {
         true
     }
 
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>>;
+    fn source_map(&self) -> Option<&Lrc<SourceMap>>;
 
     /// Formats the substitutions of the primary_span
     ///
@@ -271,7 +272,7 @@ pub trait Emitter {
     // point directly at <*macros>. Since these are often difficult to read, this
     // will change the span to point at the use site.
     fn fix_multispans_in_std_macros(&self,
-                                    source_map: &Option<Lrc<SourceMapperDyn>>,
+                                    source_map: &Option<Lrc<SourceMap>>,
                                     span: &mut MultiSpan,
                                     children: &mut Vec<SubDiagnostic>,
                                     level: &Level,
@@ -311,7 +312,7 @@ pub trait Emitter {
     // <*macros>. Since these locations are often difficult to read, we move these Spans from
     // <*macros> to their corresponding use site.
     fn fix_multispan_in_std_macros(&self,
-                                   source_map: &Option<Lrc<SourceMapperDyn>>,
+                                   source_map: &Option<Lrc<SourceMap>>,
                                    span: &mut MultiSpan,
                                    always_backtrace: bool) -> bool {
         let sm = match source_map {
@@ -397,7 +398,7 @@ pub trait Emitter {
 }
 
 impl Emitter for EmitterWriter {
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>> {
+    fn source_map(&self) -> Option<&Lrc<SourceMap>> {
         self.sm.as_ref()
     }
 
@@ -428,7 +429,7 @@ impl Emitter for EmitterWriter {
 pub struct SilentEmitter;
 
 impl Emitter for SilentEmitter {
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>> { None }
+    fn source_map(&self) -> Option<&Lrc<SourceMap>> { None }
     fn emit_diagnostic(&mut self, _: &Diagnostic) {}
 }
 
@@ -476,7 +477,7 @@ impl ColorConfig {
 /// Handles the writing of `HumanReadableErrorType::Default` and `HumanReadableErrorType::Short`
 pub struct EmitterWriter {
     dst: Destination,
-    sm: Option<Lrc<SourceMapperDyn>>,
+    sm: Option<Lrc<SourceMap>>,
     short_message: bool,
     teach: bool,
     ui_testing: bool,
@@ -495,7 +496,7 @@ pub struct FileWithAnnotatedLines {
 impl EmitterWriter {
     pub fn stderr(
         color_config: ColorConfig,
-        source_map: Option<Lrc<SourceMapperDyn>>,
+        source_map: Option<Lrc<SourceMap>>,
         short_message: bool,
         teach: bool,
         terminal_width: Option<usize>,
@@ -515,7 +516,7 @@ impl EmitterWriter {
 
     pub fn new(
         dst: Box<dyn Write + Send>,
-        source_map: Option<Lrc<SourceMapperDyn>>,
+        source_map: Option<Lrc<SourceMap>>,
         short_message: bool,
         teach: bool,
         colored: bool,
@@ -1685,7 +1686,7 @@ impl FileWithAnnotatedLines {
     /// This helps us quickly iterate over the whole message (including secondary file spans)
     pub fn collect_annotations(
         msp: &MultiSpan,
-        source_map: &Option<Lrc<SourceMapperDyn>>
+        source_map: &Option<Lrc<SourceMap>>
     ) -> Vec<FileWithAnnotatedLines> {
         fn add_annotation_to_file(file_vec: &mut Vec<FileWithAnnotatedLines>,
                                   file: Lrc<SourceFile>,
@@ -2067,7 +2068,7 @@ impl<'a> Drop for WritableDst<'a> {
 }
 
 /// Whether the original and suggested code are visually similar enough to warrant extra wording.
-pub fn is_case_difference(sm: &dyn SourceMapper, suggested: &str, sp: Span) -> bool {
+pub fn is_case_difference(sm: &SourceMap, suggested: &str, sp: Span) -> bool {
     // FIXME: this should probably be extended to also account for `FO0` → `FOO` and unicode.
     let found = sm.span_to_snippet(sp).unwrap();
     let ascii_confusables = &['c', 'f', 'i', 'k', 'o', 's', 'u', 'v', 'w', 'x', 'y', 'z'];

src/librustc_errors/json.rs

@@ -9,15 +9,15 @@
 
 // FIXME: spec the JSON output properly.
 
-use crate::source_map::{SourceMap, FilePathMapping};
+use syntax_pos::source_map::{SourceMap, FilePathMapping};
 
-use errors::registry::Registry;
-use errors::{SubDiagnostic, CodeSuggestion, SourceMapper, SourceMapperDyn};
-use errors::{DiagnosticId, Applicability};
-use errors::emitter::{Emitter, HumanReadableErrorType};
+use crate::registry::Registry;
+use crate::{SubDiagnostic, CodeSuggestion};
+use crate::{DiagnosticId, Applicability};
+use crate::emitter::{Emitter, HumanReadableErrorType};
 
 use syntax_pos::{MacroBacktrace, Span, SpanLabel, MultiSpan};
-use rustc_data_structures::sync::{self, Lrc};
+use rustc_data_structures::sync::Lrc;
 use std::io::{self, Write};
 use std::path::Path;
 use std::vec;
@@ -31,7 +31,7 @@ mod tests;
 pub struct JsonEmitter {
     dst: Box<dyn Write + Send>,
     registry: Option<Registry>,
-    sm: Lrc<dyn SourceMapper + sync::Send + sync::Sync>,
+    sm: Lrc<SourceMap>,
     pretty: bool,
     ui_testing: bool,
     json_rendered: HumanReadableErrorType,
@@ -92,7 +92,7 @@ impl JsonEmitter {
 }
 
 impl Emitter for JsonEmitter {
-    fn emit_diagnostic(&mut self, diag: &errors::Diagnostic) {
+    fn emit_diagnostic(&mut self, diag: &crate::Diagnostic) {
         let data = Diagnostic::from_errors_diagnostic(diag, self);
         let result = if self.pretty {
             writeln!(&mut self.dst, "{}", as_pretty_json(&data))
@@ -116,7 +116,7 @@ impl Emitter for JsonEmitter {
         }
     }
 
-    fn source_map(&self) -> Option<&Lrc<SourceMapperDyn>> {
+    fn source_map(&self) -> Option<&Lrc<SourceMap>> {
         Some(&self.sm)
     }
 
@@ -212,7 +212,7 @@ struct ArtifactNotification<'a> {
 }
 
 impl Diagnostic {
-    fn from_errors_diagnostic(diag: &errors::Diagnostic,
+    fn from_errors_diagnostic(diag: &crate::Diagnostic,
                                je: &JsonEmitter)
                                -> Diagnostic {
         let sugg = diag.suggestions.iter().map(|sugg| {

src/librustc_errors/json/tests.rs

@@ -1,11 +1,10 @@
 use super::*;
 
 use crate::json::JsonEmitter;
-use crate::source_map::{FilePathMapping, SourceMap};
-use crate::with_default_globals;
+use syntax_pos::source_map::{FilePathMapping, SourceMap};
 
-use errors::emitter::{ColorConfig, HumanReadableErrorType};
-use errors::Handler;
+use crate::emitter::{ColorConfig, HumanReadableErrorType};
+use crate::Handler;
 use rustc_serialize::json::decode;
 use syntax_pos::{BytePos, Span};
 
@@ -40,6 +39,13 @@ impl<T: Write> Write for Shared<T> {
     }
 }
 
+fn with_default_globals(f: impl FnOnce()) {
+    let globals = syntax_pos::Globals::new(syntax_pos::edition::DEFAULT_EDITION);
+    syntax_pos::GLOBALS.set(&globals, || {
+        syntax_pos::GLOBALS.set(&globals, f)
+    })
+}
+
 /// Test the span yields correct positions in JSON.
 fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) {
     let expected_output = TestData { spans: vec![expected_output] };

src/librustc_errors/lib.rs

@@ -18,6 +18,8 @@ use registry::Registry;
 use rustc_data_structures::sync::{self, Lrc, Lock};
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_data_structures::stable_hasher::StableHasher;
+use syntax_pos::source_map::SourceMap;
+use syntax_pos::{Loc, Span, MultiSpan};
 
 use std::borrow::Cow;
 use std::cell::Cell;
@@ -35,17 +37,7 @@ mod snippet;
 pub mod registry;
 mod styled_buffer;
 mod lock;
-
-use syntax_pos::{
-    BytePos,
-    FileLinesResult,
-    FileName,
-    Loc,
-    MultiSpan,
-    SourceFile,
-    Span,
-    SpanSnippetError,
-};
+pub mod json;
 
 pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a>>;
 
@@ -150,26 +142,12 @@ pub struct SubstitutionPart {
     pub snippet: String,
 }
 
-pub type SourceMapperDyn = dyn SourceMapper + sync::Send + sync::Sync;
-
-pub trait SourceMapper {
-    fn lookup_char_pos(&self, pos: BytePos) -> Loc;
-    fn span_to_lines(&self, sp: Span) -> FileLinesResult;
-    fn span_to_string(&self, sp: Span) -> String;
-    fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError>;
-    fn span_to_filename(&self, sp: Span) -> FileName;
-    fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option<Span>;
-    fn call_span_if_macro(&self, sp: Span) -> Span;
-    fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool;
-    fn doctest_offset_line(&self, file: &FileName, line: usize) -> usize;
-}
-
 impl CodeSuggestion {
     /// Returns the assembled code suggestions, whether they should be shown with an underline
     /// and whether the substitution only differs in capitalization.
     pub fn splice_lines(
         &self,
-        cm: &SourceMapperDyn,
+        cm: &SourceMap,
     ) -> Vec<(String, Vec<SubstitutionPart>, bool)> {
         use syntax_pos::{CharPos, Pos};
 
@@ -259,36 +237,7 @@ impl CodeSuggestion {
     }
 }
 
-/// Used as a return value to signify a fatal error occurred. (It is also
-/// used as the argument to panic at the moment, but that will eventually
-/// not be true.)
-#[derive(Copy, Clone, Debug)]
-#[must_use]
-pub struct FatalError;
-
-pub struct FatalErrorMarker;
-
-// Don't implement Send on FatalError. This makes it impossible to panic!(FatalError).
-// We don't want to invoke the panic handler and print a backtrace for fatal errors.
-impl !Send for FatalError {}
-
-impl FatalError {
-    pub fn raise(self) -> ! {
-        panic::resume_unwind(Box::new(FatalErrorMarker))
-    }
-}
-
-impl fmt::Display for FatalError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "parser fatal error")
-    }
-}
-
-impl error::Error for FatalError {
-    fn description(&self) -> &str {
-        "The parser has encountered a fatal error"
-    }
-}
+pub use syntax_pos::fatal_error::{FatalError, FatalErrorMarker};
 
 /// Signifies that the compiler died with an explicit call to `.bug`
 /// or `.span_bug` rather than a failed assertion, etc.
@@ -405,7 +354,7 @@ impl Handler {
         color_config: ColorConfig,
         can_emit_warnings: bool,
         treat_err_as_bug: Option<usize>,
-        cm: Option<Lrc<SourceMapperDyn>>,
+        cm: Option<Lrc<SourceMap>>,
     ) -> Self {
         Self::with_tty_emitter_and_flags(
             color_config,
@@ -420,7 +369,7 @@ impl Handler {
 
     pub fn with_tty_emitter_and_flags(
         color_config: ColorConfig,
-        cm: Option<Lrc<SourceMapperDyn>>,
+        cm: Option<Lrc<SourceMap>>,
         flags: HandlerFlags,
     ) -> Self {
         let emitter = Box::new(EmitterWriter::stderr(

src/librustdoc/core.rs

@@ -18,7 +18,7 @@ use syntax::ast::CRATE_NODE_ID;
 use syntax::source_map;
 use syntax::attr;
 use syntax::feature_gate::UnstableFeatures;
-use syntax::json::JsonEmitter;
+use errors::json::JsonEmitter;
 use syntax::symbol::sym;
 use syntax_pos::DUMMY_SP;
 use errors;

src/libsyntax/lib.rs

@@ -87,12 +87,10 @@ pub mod util {
     pub mod map_in_place;
 }
 
-pub mod json;
-
 pub mod ast;
 pub mod attr;
 pub mod expand;
-pub mod source_map;
+pub use syntax_pos::source_map;
 pub mod entry;
 pub mod feature_gate;
 pub mod mut_visit;

src/libsyntax_pos/Cargo.toml

@@ -18,3 +18,4 @@ arena = { path = "../libarena" }
 scoped-tls = "1.0"
 unicode-width = "0.1.4"
 cfg-if = "0.1.2"
+log = "0.4"

src/libsyntax_pos/fatal_error.rs

@@ -0,0 +1,30 @@
+/// Used as a return value to signify a fatal error occurred. (It is also
+/// used as the argument to panic at the moment, but that will eventually
+/// not be true.)
+#[derive(Copy, Clone, Debug)]
+#[must_use]
+pub struct FatalError;
+
+pub struct FatalErrorMarker;
+
+// Don't implement Send on FatalError. This makes it impossible to panic!(FatalError).
+// We don't want to invoke the panic handler and print a backtrace for fatal errors.
+impl !Send for FatalError {}
+
+impl FatalError {
+    pub fn raise(self) -> ! {
+        std::panic::resume_unwind(Box::new(FatalErrorMarker))
+    }
+}
+
+impl std::fmt::Display for FatalError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "parser fatal error")
+    }
+}
+
+impl std::error::Error for FatalError {
+    fn description(&self) -> &str {
+        "The parser has encountered a fatal error"
+    }
+}

src/libsyntax_pos/lib.rs

@@ -16,6 +16,8 @@
 
 use rustc_serialize::{Encodable, Decodable, Encoder, Decoder};
 
+pub mod source_map;
+
 pub mod edition;
 use edition::Edition;
 pub mod hygiene;
@@ -29,6 +31,7 @@ pub mod symbol;
 pub use symbol::{Symbol, sym};
 
 mod analyze_source_file;
+pub mod fatal_error;
 
 use rustc_data_structures::stable_hasher::StableHasher;
 use rustc_data_structures::sync::{Lrc, Lock};

src/libsyntax_pos/source_map.rs

@@ -7,8 +7,8 @@
 //! within the `SourceMap`, which upon request can be converted to line and column
 //! information, source code snippets, etc.
 
-pub use syntax_pos::*;
-pub use syntax_pos::hygiene::{ExpnKind, ExpnData};
+pub use crate::*;
+pub use crate::hygiene::{ExpnKind, ExpnData};
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::StableHasher;
@@ -22,8 +22,6 @@ use std::fs;
 use std::io;
 use log::debug;
 
-use errors::SourceMapper;
-
 #[cfg(test)]
 mod tests;
 
@@ -216,7 +214,7 @@ impl SourceMap {
         self.try_new_source_file(filename, src)
             .unwrap_or_else(|OffsetOverflowError| {
                 eprintln!("fatal error: rustc does not support files larger than 4GB");
-                errors::FatalError.raise()
+                crate::fatal_error::FatalError.raise()
             })
     }
 
@@ -956,37 +954,7 @@ impl SourceMap {
 
         None
     }
-}
-
-impl SourceMapper for SourceMap {
-    fn lookup_char_pos(&self, pos: BytePos) -> Loc {
-        self.lookup_char_pos(pos)
-    }
-    fn span_to_lines(&self, sp: Span) -> FileLinesResult {
-        self.span_to_lines(sp)
-    }
-    fn span_to_string(&self, sp: Span) -> String {
-        self.span_to_string(sp)
-    }
-    fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
-        self.span_to_snippet(sp)
-    }
-    fn span_to_filename(&self, sp: Span) -> FileName {
-        self.span_to_filename(sp)
-    }
-    fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option<Span> {
-        self.merge_spans(sp_lhs, sp_rhs)
-    }
-    fn call_span_if_macro(&self, sp: Span) -> Span {
-        if self.span_to_filename(sp.clone()).is_macros() {
-            let v = sp.macro_backtrace();
-            if let Some(use_site) = v.last() {
-                return use_site.call_site;
-            }
-        }
-        sp
-    }
-    fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool {
+    pub fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool {
         source_file.add_external_src(
             || match source_file.name {
                 FileName::Real(ref name) => self.file_loader.read_file(name).ok(),
@@ -994,8 +962,14 @@ impl SourceMapper for SourceMap {
             }
         )
     }
-    fn doctest_offset_line(&self, file: &FileName, line: usize) -> usize {
-        self.doctest_offset_line(file, line)
+    pub fn call_span_if_macro(&self, sp: Span) -> Span {
+        if self.span_to_filename(sp.clone()).is_macros() {
+            let v = sp.macro_backtrace();
+            if let Some(use_site) = v.last() {
+                return use_site.call_site;
+            }
+        }
+        sp
     }
 }
 

src/tools/compiletest/src/json.rs

@@ -1,4 +1,4 @@
-//! These structs are a subset of the ones found in `syntax::json`.
+//! These structs are a subset of the ones found in `rustc_errors::json`.
 //! They are only used for deserialization of JSON output provided by libtest.
 
 use crate::errors::{Error, ErrorKind};