Auto merge of #49349 - Zoxc:sync-errors, r=michaelwoerister

Make Handler more thread-safe

The use of `code_emitted` to suppress extended explanations is not thread safe. I'm not sure why we keep the documentation for errors outside `diagnostics.rs` anyway. It would be better to add a `teach` method to `DiagnosticsBuilder`, so instead of:
```
if self.tcx.sess.teach(&err.get_code().unwrap()) {
    err.note("...");
}
```
we'd use `err.teach("...")`

cc @estebank

r? @michaelwoerister

src/librustc/session/mod.rs

@@ -26,7 +26,7 @@ use util::nodemap::{FxHashSet};
 use util::common::{duration_to_secs_str, ErrorReported};
 use util::common::ProfileQueriesMsg;
 
-use rustc_data_structures::sync::{Lrc, Lock, LockCell, OneThread, Once, RwLock};
+use rustc_data_structures::sync::{self, Lrc, Lock, LockCell, OneThread, Once, RwLock};
 
 use syntax::ast::NodeId;
 use errors::{self, DiagnosticBuilder, DiagnosticId};
@@ -89,7 +89,7 @@ pub struct Session {
     /// Set of (DiagnosticId, Option<Span>, message) tuples tracking
     /// (sub)diagnostics that have been set once, but should not be set again,
     /// in order to avoid redundantly verbose output (Issue #24690, #44953).
-    pub one_time_diagnostics: RefCell<FxHashSet<(DiagnosticMessageId, Option<Span>, String)>>,
+    pub one_time_diagnostics: Lock<FxHashSet<(DiagnosticMessageId, Option<Span>, String)>>,
     pub plugin_llvm_passes: OneThread<RefCell<Vec<String>>>,
     pub plugin_attributes: OneThread<RefCell<Vec<(String, AttributeType)>>>,
     pub crate_types: Once<Vec<config::CrateType>>,
@@ -929,7 +929,7 @@ impl Session {
     }
 
     pub fn teach(&self, code: &DiagnosticId) -> bool {
-        self.opts.debugging_opts.teach && !self.parse_sess.span_diagnostic.code_emitted(code)
+        self.opts.debugging_opts.teach && self.parse_sess.span_diagnostic.must_teach(code)
     }
 
     /// Are we allowed to use features from the Rust 2018 edition?
@@ -983,7 +983,7 @@ pub fn build_session_with_codemap(
 
     let external_macro_backtrace = sopts.debugging_opts.external_macro_backtrace;
 
-    let emitter: Box<dyn Emitter> =
+    let emitter: Box<dyn Emitter + sync::Send> =
         match (sopts.error_format, emitter_dest) {
             (config::ErrorOutputType::HumanReadable(color_config), None) => Box::new(
                 EmitterWriter::stderr(
@@ -1091,7 +1091,7 @@ pub fn build_session_(
         working_dir,
         lint_store: RwLock::new(lint::LintStore::new()),
         buffered_lints: Lock::new(Some(lint::LintBuffer::new())),
-        one_time_diagnostics: RefCell::new(FxHashSet()),
+        one_time_diagnostics: Lock::new(FxHashSet()),
         plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
         plugin_attributes: OneThread::new(RefCell::new(Vec::new())),
         crate_types: Once::new(),
@@ -1188,7 +1188,7 @@ pub enum IncrCompSession {
 }
 
 pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
-    let emitter: Box<dyn Emitter> = match output {
+    let emitter: Box<dyn Emitter + sync::Send> = match output {
         config::ErrorOutputType::HumanReadable(color_config) => {
             Box::new(EmitterWriter::stderr(color_config, None, false, false))
         }
@@ -1203,7 +1203,7 @@ pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
 }
 
 pub fn early_warn(output: config::ErrorOutputType, msg: &str) {
-    let emitter: Box<dyn Emitter> = match output {
+    let emitter: Box<dyn Emitter + sync::Send> = match output {
         config::ErrorOutputType::HumanReadable(color_config) => {
             Box::new(EmitterWriter::stderr(color_config, None, false, false))
         }

src/librustc_driver/test.rs

@@ -28,7 +28,7 @@ use rustc_metadata::cstore::CStore;
 use rustc::hir::map as hir_map;
 use rustc::session::{self, config};
 use rustc::session::config::{OutputFilenames, OutputTypes};
-use rustc_data_structures::sync::Lrc;
+use rustc_data_structures::sync::{self, Lrc};
 use syntax;
 use syntax::ast;
 use syntax::abi::Abi;
@@ -88,13 +88,13 @@ impl Emitter for ExpectErrorEmitter {
     }
 }
 
-fn errors(msgs: &[&str]) -> (Box<Emitter + Send>, usize) {
+fn errors(msgs: &[&str]) -> (Box<Emitter + sync::Send>, usize) {
     let v = msgs.iter().map(|m| m.to_string()).collect();
-    (box ExpectErrorEmitter { messages: v } as Box<Emitter + Send>, msgs.len())
+    (box ExpectErrorEmitter { messages: v } as Box<Emitter + sync::Send>, msgs.len())
 }
 
 fn test_env<F>(source_string: &str,
-               args: (Box<Emitter + Send>, usize),
+               args: (Box<Emitter + sync::Send>, usize),
                body: F)
     where F: FnOnce(Env)
 {
@@ -104,7 +104,7 @@ fn test_env<F>(source_string: &str,
 }
 
 fn test_env_impl<F>(source_string: &str,
-                    (emitter, expected_err_count): (Box<Emitter + Send>, usize),
+                    (emitter, expected_err_count): (Box<Emitter + sync::Send>, usize),
                     body: F)
     where F: FnOnce(Env)
 {

src/librustc_errors/lib.rs

@@ -33,12 +33,12 @@ use self::Level::*;
 
 use emitter::{Emitter, EmitterWriter};
 
-use rustc_data_structures::sync::{self, Lrc};
+use rustc_data_structures::sync::{self, Lrc, Lock, LockCell};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stable_hasher::StableHasher;
 
 use std::borrow::Cow;
-use std::cell::{RefCell, Cell};
+use std::cell::Cell;
 use std::{error, fmt};
 use std::sync::atomic::AtomicUsize;
 use std::sync::atomic::Ordering::SeqCst;
@@ -262,19 +262,22 @@ pub struct Handler {
     pub flags: HandlerFlags,
 
     err_count: AtomicUsize,
-    emitter: RefCell<Box<Emitter>>,
-    continue_after_error: Cell<bool>,
-    delayed_span_bug: RefCell<Option<Diagnostic>>,
+    emitter: Lock<Box<Emitter + sync::Send>>,
+    continue_after_error: LockCell<bool>,
+    delayed_span_bug: Lock<Option<Diagnostic>>,
 
     // This set contains the `DiagnosticId` of all emitted diagnostics to avoid
     // emitting the same diagnostic with extended help (`--teach`) twice, which
     // would be uneccessary repetition.
-    tracked_diagnostic_codes: RefCell<FxHashSet<DiagnosticId>>,
+    taught_diagnostics: Lock<FxHashSet<DiagnosticId>>,
+
+    /// Used to suggest rustc --explain <error code>
+    emitted_diagnostic_codes: Lock<FxHashSet<DiagnosticId>>,
 
     // This set contains a hash of every diagnostic that has been emitted by
     // this handler. These hashes is used to avoid emitting the same error
     // twice.
-    emitted_diagnostics: RefCell<FxHashSet<u128>>,
+    emitted_diagnostics: Lock<FxHashSet<u128>>,
 }
 
 fn default_track_diagnostic(_: &Diagnostic) {}
@@ -315,7 +318,7 @@ impl Handler {
 
     pub fn with_emitter(can_emit_warnings: bool,
                         treat_err_as_bug: bool,
-                        e: Box<Emitter>)
+                        e: Box<Emitter + sync::Send>)
                         -> Handler {
         Handler::with_emitter_and_flags(
             e,
@@ -326,15 +329,16 @@ impl Handler {
             })
     }
 
-    pub fn with_emitter_and_flags(e: Box<Emitter>, flags: HandlerFlags) -> Handler {
+    pub fn with_emitter_and_flags(e: Box<Emitter + sync::Send>, flags: HandlerFlags) -> Handler {
         Handler {
             flags,
             err_count: AtomicUsize::new(0),
-            emitter: RefCell::new(e),
-            continue_after_error: Cell::new(true),
-            delayed_span_bug: RefCell::new(None),
-            tracked_diagnostic_codes: RefCell::new(FxHashSet()),
-            emitted_diagnostics: RefCell::new(FxHashSet()),
+            emitter: Lock::new(e),
+            continue_after_error: LockCell::new(true),
+            delayed_span_bug: Lock::new(None),
+            taught_diagnostics: Lock::new(FxHashSet()),
+            emitted_diagnostic_codes: Lock::new(FxHashSet()),
+            emitted_diagnostics: Lock::new(FxHashSet()),
         }
     }
 
@@ -348,7 +352,7 @@ impl Handler {
     /// tools that want to reuse a `Parser` cleaning the previously emitted diagnostics as well as
     /// the overall count of emitted error diagnostics.
     pub fn reset_err_count(&self) {
-        self.emitted_diagnostics.replace(FxHashSet());
+        *self.emitted_diagnostics.borrow_mut() = FxHashSet();
         self.err_count.store(0, SeqCst);
     }
 
@@ -568,10 +572,10 @@ impl Handler {
         let _ = self.fatal(&s);
 
         let can_show_explain = self.emitter.borrow().should_show_explain();
-        let are_there_diagnostics = !self.tracked_diagnostic_codes.borrow().is_empty();
+        let are_there_diagnostics = !self.emitted_diagnostic_codes.borrow().is_empty();
         if can_show_explain && are_there_diagnostics {
             let mut error_codes =
-                self.tracked_diagnostic_codes.borrow()
+                self.emitted_diagnostic_codes.borrow()
                                              .clone()
                                              .into_iter()
                                              .filter_map(|x| match x {
@@ -630,12 +634,13 @@ impl Handler {
         }
     }
 
-    /// `true` if a diagnostic with this code has already been emitted in this handler.
+    /// `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 code_emitted(&self, code: &DiagnosticId) -> bool {
-        self.tracked_diagnostic_codes.borrow().contains(code)
+    pub fn must_teach(&self, code: &DiagnosticId) -> bool {
+        self.taught_diagnostics.borrow_mut().insert(code.clone())
     }
 
     pub fn force_print_db(&self, mut db: DiagnosticBuilder) {
@@ -651,7 +656,7 @@ impl Handler {
         });
 
         if let Some(ref code) = diagnostic.code {
-            self.tracked_diagnostic_codes.borrow_mut().insert(code.clone());
+            self.emitted_diagnostic_codes.borrow_mut().insert(code.clone());
         }
 
         let diagnostic_hash = {

src/librustdoc/core.rs

@@ -35,7 +35,7 @@ use errors::emitter::{Emitter, EmitterWriter};
 
 use std::cell::{RefCell, Cell};
 use std::mem;
-use rustc_data_structures::sync::Lrc;
+use rustc_data_structures::sync::{self, Lrc};
 use std::rc::Rc;
 use std::path::PathBuf;
 
@@ -163,7 +163,7 @@ pub fn run_core(search_paths: SearchPaths,
     };
 
     let codemap = Lrc::new(codemap::CodeMap::new(sessopts.file_path_mapping()));
-    let emitter: Box<dyn Emitter> = match error_format {
+    let emitter: Box<dyn Emitter + sync::Send> = match error_format {
         ErrorOutputType::HumanReadable(color_config) => Box::new(
             EmitterWriter::stderr(
                 color_config,