Auto merge of #48904 - Zoxc:code-and-file-maps, r=michaelwoerister

Make CodeMap and FileMap thread-safe

r? @michaelwoerister

8 files changed, 112 insertions, 99 deletions

diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs
index 52f43fbed7b..289bc753d7f 100644
--- a/src/librustc/ich/impls_syntax.rs
+++ b/src/librustc/ich/impls_syntax.rs
@@ -417,24 +417,27 @@ impl<'a> HashStable<StableHashingContext<'a>> for FileMap {
         src_hash.hash_stable(hcx, hasher);
 
         // We only hash the relative position within this filemap
-        let lines = lines.borrow();
-        lines.len().hash_stable(hcx, hasher);
-        for &line in lines.iter() {
-            stable_byte_pos(line, start_pos).hash_stable(hcx, hasher);
-        }
+        lines.with_lock(|lines| {
+            lines.len().hash_stable(hcx, hasher);
+            for &line in lines.iter() {
+                stable_byte_pos(line, start_pos).hash_stable(hcx, hasher);
+            }
+        });
 
         // We only hash the relative position within this filemap
-        let multibyte_chars = multibyte_chars.borrow();
-        multibyte_chars.len().hash_stable(hcx, hasher);
-        for &char_pos in multibyte_chars.iter() {
-            stable_multibyte_char(char_pos, start_pos).hash_stable(hcx, hasher);
-        }
+        multibyte_chars.with_lock(|multibyte_chars| {
+            multibyte_chars.len().hash_stable(hcx, hasher);
+            for &char_pos in multibyte_chars.iter() {
+                stable_multibyte_char(char_pos, start_pos).hash_stable(hcx, hasher);
+            }
+        });
 
-        let non_narrow_chars = non_narrow_chars.borrow();
-        non_narrow_chars.len().hash_stable(hcx, hasher);
-        for &char_pos in non_narrow_chars.iter() {
-            stable_non_narrow_char(char_pos, start_pos).hash_stable(hcx, hasher);
-        }
+        non_narrow_chars.with_lock(|non_narrow_chars| {
+            non_narrow_chars.len().hash_stable(hcx, hasher);
+            for &char_pos in non_narrow_chars.iter() {
+                stable_non_narrow_char(char_pos, start_pos).hash_stable(hcx, hasher);
+            }
+        });
     }
 }
 
diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs
index 5a1983bfec6..f1f3a0519bb 100644
--- a/src/librustc_driver/lib.rs
+++ b/src/librustc_driver/lib.rs
@@ -444,7 +444,7 @@ fn get_trans_sysroot(backend_name: &str) -> fn() -> Box<TransCrate> {
 // The FileLoader provides a way to load files from sources other than the file system.
 pub fn run_compiler<'a>(args: &[String],
                         callbacks: &mut CompilerCalls<'a>,
-                        file_loader: Option<Box<FileLoader + 'static>>,
+                        file_loader: Option<Box<FileLoader + Send + Sync + 'static>>,
                         emitter_dest: Option<Box<Write + Send>>)
                         -> (CompileResult, Option<Session>)
 {
@@ -455,7 +455,7 @@ pub fn run_compiler<'a>(args: &[String],
 
 fn run_compiler_impl<'a>(args: &[String],
                          callbacks: &mut CompilerCalls<'a>,
-                         file_loader: Option<Box<FileLoader + 'static>>,
+                         file_loader: Option<Box<FileLoader + Send + Sync + 'static>>,
                          emitter_dest: Option<Box<Write + Send>>)
                          -> (CompileResult, Option<Session>)
 {
diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
index 3b6e6db7f46..ca5d3f55a0f 100644
--- a/src/librustc_errors/emitter.rs
+++ b/src/librustc_errors/emitter.rs
@@ -12,7 +12,7 @@ use self::Destination::*;
 
 use syntax_pos::{DUMMY_SP, FileMap, Span, MultiSpan};
 
-use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper, DiagnosticId};
+use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapperDyn, DiagnosticId};
 use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
 use styled_buffer::StyledBuffer;
 
@@ -120,7 +120,7 @@ impl ColorConfig {
 
 pub struct EmitterWriter {
     dst: Destination,
-    cm: Option<Lrc<CodeMapper>>,
+    cm: Option<Lrc<CodeMapperDyn>>,
     short_message: bool,
     teach: bool,
     ui_testing: bool,
@@ -134,7 +134,7 @@ struct FileWithAnnotatedLines {
 
 impl EmitterWriter {
     pub fn stderr(color_config: ColorConfig,
-                  code_map: Option<Lrc<CodeMapper>>,
+                  code_map: Option<Lrc<CodeMapperDyn>>,
                   short_message: bool,
                   teach: bool)
                   -> EmitterWriter {
@@ -149,7 +149,7 @@ impl EmitterWriter {
     }
 
     pub fn new(dst: Box<Write + Send>,
-               code_map: Option<Lrc<CodeMapper>>,
+               code_map: Option<Lrc<CodeMapperDyn>>,
                short_message: bool,
                teach: bool)
                -> EmitterWriter {
@@ -1195,8 +1195,6 @@ impl EmitterWriter {
                                level: &Level,
                                max_line_num_len: usize)
                                -> io::Result<()> {
-        use std::borrow::Borrow;
-
         if let Some(ref cm) = self.cm {
             let mut buffer = StyledBuffer::new();
 
@@ -1213,7 +1211,7 @@ impl EmitterWriter {
                                Some(Style::HeaderMsg));
 
             // Render the replacements for each suggestion
-            let suggestions = suggestion.splice_lines(cm.borrow());
+            let suggestions = suggestion.splice_lines(&**cm);
 
             let mut row_num = 2;
             for &(ref complete, ref parts) in suggestions.iter().take(MAX_SUGGESTIONS) {
diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs
index 7148969191f..a25c3668bb1 100644
--- a/src/librustc_errors/lib.rs
+++ b/src/librustc_errors/lib.rs
@@ -36,7 +36,7 @@ use self::Level::*;
 
 use emitter::{Emitter, EmitterWriter};
 
-use rustc_data_structures::sync::Lrc;
+use rustc_data_structures::sync::{self, Lrc};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stable_hasher::StableHasher;
 
@@ -106,6 +106,8 @@ pub struct SubstitutionPart {
     pub snippet: String,
 }
 
+pub type CodeMapperDyn = CodeMapper + sync::Send + sync::Sync;
+
 pub trait CodeMapper {
     fn lookup_char_pos(&self, pos: BytePos) -> Loc;
     fn span_to_lines(&self, sp: Span) -> FileLinesResult;
@@ -119,7 +121,8 @@ pub trait CodeMapper {
 
 impl CodeSuggestion {
     /// Returns the assembled code suggestions and whether they should be shown with an underline.
-    pub fn splice_lines(&self, cm: &CodeMapper) -> Vec<(String, Vec<SubstitutionPart>)> {
+    pub fn splice_lines(&self, cm: &CodeMapperDyn)
+                        -> Vec<(String, Vec<SubstitutionPart>)> {
         use syntax_pos::{CharPos, Loc, Pos};
 
         fn push_trailing(buf: &mut String,
@@ -290,7 +293,7 @@ impl Handler {
     pub fn with_tty_emitter(color_config: ColorConfig,
                             can_emit_warnings: bool,
                             treat_err_as_bug: bool,
-                            cm: Option<Lrc<CodeMapper>>)
+                            cm: Option<Lrc<CodeMapperDyn>>)
                             -> Handler {
         Handler::with_tty_emitter_and_flags(
             color_config,
@@ -303,7 +306,7 @@ impl Handler {
     }
 
     pub fn with_tty_emitter_and_flags(color_config: ColorConfig,
-                                      cm: Option<Lrc<CodeMapper>>,
+                                      cm: Option<Lrc<CodeMapperDyn>>,
                                       flags: HandlerFlags)
                                       -> Handler {
         let emitter = Box::new(EmitterWriter::stderr(color_config, cm, false, false));
diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index 951f8a871ca..a1aec052088 100644
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -24,8 +24,7 @@ pub use self::ExpnFormat::*;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::StableHasher;
-use rustc_data_structures::sync::Lrc;
-use std::cell::{RefCell, Ref};
+use rustc_data_structures::sync::{Lrc, Lock, LockGuard};
 use std::cmp;
 use std::hash::Hash;
 use std::path::{Path, PathBuf};
@@ -125,13 +124,17 @@ impl StableFilemapId {
 // CodeMap
 //
 
+pub(super) struct CodeMapFiles {
+    pub(super) file_maps: Vec<Lrc<FileMap>>,
+    stable_id_to_filemap: FxHashMap<StableFilemapId, Lrc<FileMap>>
+}
+
 pub struct CodeMap {
-    pub(super) files: RefCell<Vec<Lrc<FileMap>>>,
-    file_loader: Box<FileLoader>,
+    pub(super) files: Lock<CodeMapFiles>,
+    file_loader: Box<FileLoader + Sync + Send>,
     // This is used to apply the file path remapping as specified via
     // --remap-path-prefix to all FileMaps allocated within this CodeMap.
     path_mapping: FilePathMapping,
-    stable_id_to_filemap: RefCell<FxHashMap<StableFilemapId, Lrc<FileMap>>>,
     /// In case we are in a doctest, replace all file names with the PathBuf,
     /// and add the given offsets to the line info
     doctest_offset: Option<(FileName, isize)>,
@@ -140,10 +143,12 @@ pub struct CodeMap {
 impl CodeMap {
     pub fn new(path_mapping: FilePathMapping) -> CodeMap {
         CodeMap {
-            files: RefCell::new(Vec::new()),
+            files: Lock::new(CodeMapFiles {
+                file_maps: Vec::new(),
+                stable_id_to_filemap: FxHashMap(),
+            }),
             file_loader: Box::new(RealFileLoader),
             path_mapping,
-            stable_id_to_filemap: RefCell::new(FxHashMap()),
             doctest_offset: None,
         }
     }
@@ -157,14 +162,16 @@ impl CodeMap {
 
     }
 
-    pub fn with_file_loader(file_loader: Box<FileLoader>,
+    pub fn with_file_loader(file_loader: Box<FileLoader + Sync + Send>,
                             path_mapping: FilePathMapping)
                             -> CodeMap {
         CodeMap {
-            files: RefCell::new(Vec::new()),
-            file_loader,
+            files: Lock::new(CodeMapFiles {
+                file_maps: Vec::new(),
+                stable_id_to_filemap: FxHashMap(),
+            }),
+            file_loader: file_loader,
             path_mapping,
-            stable_id_to_filemap: RefCell::new(FxHashMap()),
             doctest_offset: None,
         }
     }
@@ -187,17 +194,16 @@ impl CodeMap {
         Ok(self.new_filemap(filename, src))
     }
 
-    pub fn files(&self) -> Ref<Vec<Lrc<FileMap>>> {
-        self.files.borrow()
+    pub fn files(&self) -> LockGuard<Vec<Lrc<FileMap>>> {
+        LockGuard::map(self.files.borrow(), |files| &mut files.file_maps)
     }
 
     pub fn filemap_by_stable_id(&self, stable_id: StableFilemapId) -> Option<Lrc<FileMap>> {
-        self.stable_id_to_filemap.borrow().get(&stable_id).map(|fm| fm.clone())
+        self.files.borrow().stable_id_to_filemap.get(&stable_id).map(|fm| fm.clone())
     }
 
     fn next_start_pos(&self) -> usize {
-        let files = self.files.borrow();
-        match files.last() {
+        match self.files.borrow().file_maps.last() {
             None => 0,
             // Add one so there is some space between files. This lets us distinguish
             // positions in the codemap, even in the presence of zero-length files.
@@ -207,9 +213,9 @@ impl CodeMap {
 
     /// Creates a new filemap without setting its line information. If you don't
     /// intend to set the line information yourself, you should use new_filemap_and_lines.
+    /// This does not ensure that only one FileMap exists per file name.
     pub fn new_filemap(&self, filename: FileName, src: String) -> Lrc<FileMap> {
         let start_pos = self.next_start_pos();
-        let mut files = self.files.borrow_mut();
 
         // The path is used to determine the directory for loading submodules and
         // include files, so it must be before remapping.
@@ -233,16 +239,16 @@ impl CodeMap {
             Pos::from_usize(start_pos),
         ));
 
-        files.push(filemap.clone());
+        let mut files = self.files.borrow_mut();
 
-        self.stable_id_to_filemap
-            .borrow_mut()
-            .insert(StableFilemapId::new(&filemap), filemap.clone());
+        files.file_maps.push(filemap.clone());
+        files.stable_id_to_filemap.insert(StableFilemapId::new(&filemap), filemap.clone());
 
         filemap
     }
 
     /// Creates a new filemap and sets its line information.
+    /// This does not ensure that only one FileMap exists per file name.
     pub fn new_filemap_and_lines(&self, filename: &Path, src: &str) -> Lrc<FileMap> {
         let fm = self.new_filemap(filename.to_owned().into(), src.to_owned());
         let mut byte_pos: u32 = fm.start_pos.0;
@@ -273,7 +279,6 @@ impl CodeMap {
                                 mut file_local_non_narrow_chars: Vec<NonNarrowChar>)
                                 -> Lrc<FileMap> {
         let start_pos = self.next_start_pos();
-        let mut files = self.files.borrow_mut();
 
         let end_pos = Pos::from_usize(start_pos + source_len);
         let start_pos = Pos::from_usize(start_pos);
@@ -297,20 +302,19 @@ impl CodeMap {
             crate_of_origin,
             src: None,
             src_hash,
-            external_src: RefCell::new(ExternalSource::AbsentOk),
+            external_src: Lock::new(ExternalSource::AbsentOk),
             start_pos,
             end_pos,
-            lines: RefCell::new(file_local_lines),
-            multibyte_chars: RefCell::new(file_local_multibyte_chars),
-            non_narrow_chars: RefCell::new(file_local_non_narrow_chars),
+            lines: Lock::new(file_local_lines),
+            multibyte_chars: Lock::new(file_local_multibyte_chars),
+            non_narrow_chars: Lock::new(file_local_non_narrow_chars),
             name_hash,
         });
 
-        files.push(filemap.clone());
+        let mut files = self.files.borrow_mut();
 
-        self.stable_id_to_filemap
-            .borrow_mut()
-            .insert(StableFilemapId::new(&filemap), filemap.clone());
+        files.file_maps.push(filemap.clone());
+        files.stable_id_to_filemap.insert(StableFilemapId::new(&filemap), filemap.clone());
 
         filemap
     }
@@ -401,8 +405,7 @@ impl CodeMap {
     pub fn lookup_line(&self, pos: BytePos) -> Result<FileMapAndLine, Lrc<FileMap>> {
         let idx = self.lookup_filemap_idx(pos);
 
-        let files = self.files.borrow();
-        let f = (*files)[idx].clone();
+        let f = (*self.files.borrow().file_maps)[idx].clone();
 
         match f.lookup_line(pos) {
             Some(line) => Ok(FileMapAndLine { fm: f, line: line }),
@@ -456,7 +459,7 @@ impl CodeMap {
     }
 
     pub fn span_to_string(&self, sp: Span) -> String {
-        if self.files.borrow().is_empty() && sp.source_equal(&DUMMY_SP) {
+        if self.files.borrow().file_maps.is_empty() && sp.source_equal(&DUMMY_SP) {
             return "no-location".to_string();
         }
 
@@ -799,7 +802,7 @@ impl CodeMap {
     }
 
     pub fn get_filemap(&self, filename: &FileName) -> Option<Lrc<FileMap>> {
-        for fm in self.files.borrow().iter() {
+        for fm in self.files.borrow().file_maps.iter() {
             if *filename == fm.name {
                 return Some(fm.clone());
             }
@@ -810,7 +813,7 @@ impl CodeMap {
     /// For a global BytePos compute the local offset within the containing FileMap
     pub fn lookup_byte_offset(&self, bpos: BytePos) -> FileMapAndBytePos {
         let idx = self.lookup_filemap_idx(bpos);
-        let fm = (*self.files.borrow())[idx].clone();
+        let fm = (*self.files.borrow().file_maps)[idx].clone();
         let offset = bpos - fm.start_pos;
         FileMapAndBytePos {fm: fm, pos: offset}
     }
@@ -818,8 +821,7 @@ impl CodeMap {
     /// Converts an absolute BytePos to a CharPos relative to the filemap.
     pub fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos {
         let idx = self.lookup_filemap_idx(bpos);
-        let files = self.files.borrow();
-        let map = &(*files)[idx];
+        let map = &(*self.files.borrow().file_maps)[idx];
 
         // The number of extra bytes due to multibyte chars in the FileMap
         let mut total_extra_bytes = 0;
@@ -845,7 +847,7 @@ impl CodeMap {
     // Return the index of the filemap (in self.files) which contains pos.
     pub fn lookup_filemap_idx(&self, pos: BytePos) -> usize {
         let files = self.files.borrow();
-        let files = &*files;
+        let files = &files.file_maps;
         let count = files.len();
 
         // Binary search for the filemap.
diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs
index eed3c691405..b4f34fb12e3 100644
--- a/src/libsyntax/json.rs
+++ b/src/libsyntax/json.rs
@@ -26,7 +26,7 @@ use errors::{DiagnosticBuilder, SubDiagnostic, CodeSuggestion, CodeMapper};
 use errors::DiagnosticId;
 use errors::emitter::{Emitter, EmitterWriter};
 
-use rustc_data_structures::sync::Lrc;
+use rustc_data_structures::sync::{self, Lrc};
 use std::io::{self, Write};
 use std::vec;
 use std::sync::{Arc, Mutex};
@@ -36,7 +36,7 @@ use rustc_serialize::json::{as_json, as_pretty_json};
 pub struct JsonEmitter {
     dst: Box<Write + Send>,
     registry: Option<Registry>,
-    cm: Lrc<CodeMapper + 'static>,
+    cm: Lrc<CodeMapper + sync::Send + sync::Sync>,
     pretty: bool,
     /// Whether "approximate suggestions" are enabled in the config
     approximate_suggestions: bool,
diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs
index d0075c89656..815ba49a60a 100644
--- a/src/libsyntax/parse/lexer/mod.rs
+++ b/src/libsyntax/parse/lexer/mod.rs
@@ -611,7 +611,7 @@ impl<'a> StringReader<'a> {
                 // I guess this is the only way to figure out if
                 // we're at the beginning of the file...
                 let cmap = CodeMap::new(FilePathMapping::empty());
-                cmap.files.borrow_mut().push(self.filemap.clone());
+                cmap.files.borrow_mut().file_maps.push(self.filemap.clone());
                 let loc = cmap.lookup_char_pos_adj(self.pos);
                 debug!("Skipping a shebang");
                 if loc.line == 1 && loc.col == CharPos(0) {
diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
index bec46ff3d79..4711d43bfab 100644
--- a/src/libsyntax_pos/lib.rs
+++ b/src/libsyntax_pos/lib.rs
@@ -27,7 +27,7 @@
 #![feature(specialization)]
 
 use std::borrow::Cow;
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::cmp::{self, Ordering};
 use std::fmt;
 use std::hash::{Hasher, Hash};
@@ -699,17 +699,17 @@ pub struct FileMap {
     pub src_hash: u128,
     /// The external source code (used for external crates, which will have a `None`
     /// value as `self.src`.
-    pub external_src: RefCell<ExternalSource>,
+    pub external_src: Lock<ExternalSource>,
     /// The start position of this source in the CodeMap
     pub start_pos: BytePos,
     /// The end position of this source in the CodeMap
     pub end_pos: BytePos,
     /// Locations of lines beginnings in the source code
-    pub lines: RefCell<Vec<BytePos>>,
+    pub lines: Lock<Vec<BytePos>>,
     /// Locations of multi-byte characters in the source code
-    pub multibyte_chars: RefCell<Vec<MultiByteChar>>,
+    pub multibyte_chars: Lock<Vec<MultiByteChar>>,
     /// Width of characters that are not narrow in the source code
-    pub non_narrow_chars: RefCell<Vec<NonNarrowChar>>,
+    pub non_narrow_chars: Lock<Vec<NonNarrowChar>>,
     /// A hash of the filename, used for speeding up the incr. comp. hashing.
     pub name_hash: u128,
 }
@@ -839,10 +839,10 @@ impl Decodable for FileMap {
                 end_pos,
                 src: None,
                 src_hash,
-                external_src: RefCell::new(ExternalSource::AbsentOk),
-                lines: RefCell::new(lines),
-                multibyte_chars: RefCell::new(multibyte_chars),
-                non_narrow_chars: RefCell::new(non_narrow_chars),
+                external_src: Lock::new(ExternalSource::AbsentOk),
+                lines: Lock::new(lines),
+                multibyte_chars: Lock::new(multibyte_chars),
+                non_narrow_chars: Lock::new(non_narrow_chars),
                 name_hash,
             })
         })
@@ -882,12 +882,12 @@ impl FileMap {
             crate_of_origin: 0,
             src: Some(Lrc::new(src)),
             src_hash,
-            external_src: RefCell::new(ExternalSource::Unneeded),
+            external_src: Lock::new(ExternalSource::Unneeded),
             start_pos,
             end_pos: Pos::from_usize(end_pos),
-            lines: RefCell::new(Vec::new()),
-            multibyte_chars: RefCell::new(Vec::new()),
-            non_narrow_chars: RefCell::new(Vec::new()),
+            lines: Lock::new(Vec::new()),
+            multibyte_chars: Lock::new(Vec::new()),
+            non_narrow_chars: Lock::new(Vec::new()),
             name_hash,
         }
     }
@@ -919,19 +919,24 @@ impl FileMap {
         if *self.external_src.borrow() == ExternalSource::AbsentOk {
             let src = get_src();
             let mut external_src = self.external_src.borrow_mut();
-            if let Some(src) = src {
-                let mut hasher: StableHasher<u128> = StableHasher::new();
-                hasher.write(src.as_bytes());
-
-                if hasher.finish() == self.src_hash {
-                    *external_src = ExternalSource::Present(src);
-                    return true;
+            // Check that no-one else have provided the source while we were getting it
+            if *external_src == ExternalSource::AbsentOk {
+                if let Some(src) = src {
+                    let mut hasher: StableHasher<u128> = StableHasher::new();
+                    hasher.write(src.as_bytes());
+
+                    if hasher.finish() == self.src_hash {
+                        *external_src = ExternalSource::Present(src);
+                        return true;
+                    }
+                } else {
+                    *external_src = ExternalSource::AbsentErr;
                 }
+
+                false
             } else {
-                *external_src = ExternalSource::AbsentErr;
+                self.src.is_some() || external_src.get_source().is_some()
             }
-
-            false
         } else {
             self.src.is_some() || self.external_src.borrow().get_source().is_some()
         }
@@ -951,14 +956,16 @@ impl FileMap {
             }
         }
 
-        let lines = self.lines.borrow();
-        let line = if let Some(line) = lines.get(line_number) {
-            line
-        } else {
-            return None;
+        let begin = {
+            let lines = self.lines.borrow();
+            let line = if let Some(line) = lines.get(line_number) {
+                line
+            } else {
+                return None;
+            };
+            let begin: BytePos = *line - self.start_pos;
+            begin.to_usize()
         };
-        let begin: BytePos = *line - self.start_pos;
-        let begin = begin.to_usize();
 
         if let Some(ref src) = self.src {
             Some(Cow::from(get_until_newline(src, begin)))