From 26fe97f1f90f71613b648e822e663ec52832c51e Mon Sep 17 00:00:00 2001 From: John Kåre Alsaker Date: Sat, 3 Mar 2018 06:21:27 +0100 Subject: Require a thread-safe file loader --- src/libsyntax/codemap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index c340f1b8c8a..6a5a180fc0a 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -127,7 +127,7 @@ impl StableFilemapId { pub struct CodeMap { pub(super) files: RefCell>>, - file_loader: Box, + file_loader: Box, // 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, @@ -157,7 +157,7 @@ impl CodeMap { } - pub fn with_file_loader(file_loader: Box, + pub fn with_file_loader(file_loader: Box, path_mapping: FilePathMapping) -> CodeMap { CodeMap { -- cgit 1.4.1-3-g733a5 From a857e6003e3f1ee1023bb8ec50cf652164bf5e11 Mon Sep 17 00:00:00 2001 From: John Kåre Alsaker Date: Sat, 10 Mar 2018 06:40:25 +0100 Subject: Make CodeMap thread-safe --- src/libsyntax/codemap.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 6a5a180fc0a..829ba386d35 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}; @@ -126,12 +125,12 @@ impl StableFilemapId { // pub struct CodeMap { - pub(super) files: RefCell>>, + pub(super) files: Lock>>, file_loader: Box, // 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>>, + stable_id_to_filemap: Lock>>, /// 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 +139,10 @@ pub struct CodeMap { impl CodeMap { pub fn new(path_mapping: FilePathMapping) -> CodeMap { CodeMap { - files: RefCell::new(Vec::new()), + files: Lock::new(Vec::new()), file_loader: Box::new(RealFileLoader), path_mapping, - stable_id_to_filemap: RefCell::new(FxHashMap()), + stable_id_to_filemap: Lock::new(FxHashMap()), doctest_offset: None, } } @@ -161,10 +160,10 @@ impl CodeMap { path_mapping: FilePathMapping) -> CodeMap { CodeMap { - files: RefCell::new(Vec::new()), - file_loader, + files: Lock::new(Vec::new()), + file_loader: file_loader, path_mapping, - stable_id_to_filemap: RefCell::new(FxHashMap()), + stable_id_to_filemap: Lock::new(FxHashMap()), doctest_offset: None, } } @@ -187,7 +186,7 @@ impl CodeMap { Ok(self.new_filemap(filename, src)) } - pub fn files(&self) -> Ref>> { + pub fn files(&self) -> LockGuard>> { self.files.borrow() } @@ -209,7 +208,6 @@ impl CodeMap { /// intend to set the line information yourself, you should use new_filemap_and_lines. pub fn new_filemap(&self, filename: FileName, src: String) -> Lrc { 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,7 +231,7 @@ impl CodeMap { Pos::from_usize(start_pos), )); - files.push(filemap.clone()); + self.files.borrow_mut().push(filemap.clone()); self.stable_id_to_filemap .borrow_mut() @@ -273,7 +271,6 @@ impl CodeMap { mut file_local_non_narrow_chars: Vec) -> Lrc { 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,16 +294,16 @@ 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()); + self.files.borrow_mut().push(filemap.clone()); self.stable_id_to_filemap .borrow_mut() @@ -401,8 +398,7 @@ impl CodeMap { pub fn lookup_line(&self, pos: BytePos) -> Result> { let idx = self.lookup_filemap_idx(pos); - let files = self.files.borrow(); - let f = (*files)[idx].clone(); + let f = (*self.files.borrow())[idx].clone(); match f.lookup_line(pos) { Some(line) => Ok(FileMapAndLine { fm: f, line: line }), @@ -810,8 +806,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())[idx]; // The number of extra bytes due to multibyte chars in the FileMap let mut total_extra_bytes = 0; -- cgit 1.4.1-3-g733a5 From 8395ce9451c901a4b5ce3afd916bd20785e6db7f Mon Sep 17 00:00:00 2001 From: John Kåre Alsaker Date: Sat, 3 Mar 2018 06:26:02 +0100 Subject: Require the code mapper to be thread-safe --- src/librustc_errors/emitter.rs | 12 +++++------- src/librustc_errors/lib.rs | 11 +++++++---- src/libsyntax/json.rs | 4 ++-- 3 files changed, 14 insertions(+), 13 deletions(-) (limited to 'src/libsyntax') 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>, + cm: Option>, 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>, + code_map: Option>, short_message: bool, teach: bool) -> EmitterWriter { @@ -149,7 +149,7 @@ impl EmitterWriter { } pub fn new(dst: Box, - code_map: Option>, + code_map: Option>, 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)> { + pub fn splice_lines(&self, cm: &CodeMapperDyn) + -> Vec<(String, Vec)> { 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>) + cm: Option>) -> 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>, + cm: Option>, flags: HandlerFlags) -> Handler { let emitter = Box::new(EmitterWriter::stderr(color_config, cm, false, false)); 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, registry: Option, - cm: Lrc, + cm: Lrc, pretty: bool, /// Whether "approximate suggestions" are enabled in the config approximate_suggestions: bool, -- cgit 1.4.1-3-g733a5 From 65b49902538b319f9fb07532beff9d02efd3197f Mon Sep 17 00:00:00 2001 From: John Kåre Alsaker Date: Wed, 14 Mar 2018 18:11:37 +0100 Subject: Use a single Lock for CodeMap.stable_id_to_filemap and CodeMap.files --- src/libsyntax/codemap.rs | 55 ++++++++++++++++++++++------------------ src/libsyntax/parse/lexer/mod.rs | 2 +- 2 files changed, 32 insertions(+), 25 deletions(-) (limited to 'src/libsyntax') diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 829ba386d35..7027fdfa2fe 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -124,13 +124,17 @@ impl StableFilemapId { // CodeMap // +pub(super) struct CodeMapFiles { + pub(super) file_maps: Vec>, + stable_id_to_filemap: FxHashMap> +} + pub struct CodeMap { - pub(super) files: Lock>>, + pub(super) files: Lock, file_loader: Box, // 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: Lock>>, /// 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)>, @@ -139,10 +143,12 @@ pub struct CodeMap { impl CodeMap { pub fn new(path_mapping: FilePathMapping) -> CodeMap { CodeMap { - files: Lock::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: Lock::new(FxHashMap()), doctest_offset: None, } } @@ -160,10 +166,12 @@ impl CodeMap { path_mapping: FilePathMapping) -> CodeMap { CodeMap { - files: Lock::new(Vec::new()), + files: Lock::new(CodeMapFiles { + file_maps: Vec::new(), + stable_id_to_filemap: FxHashMap(), + }), file_loader: file_loader, path_mapping, - stable_id_to_filemap: Lock::new(FxHashMap()), doctest_offset: None, } } @@ -187,16 +195,15 @@ impl CodeMap { } pub fn files(&self) -> LockGuard>> { - self.files.borrow() + LockGuard::map(self.files.borrow(), |files| &mut files.file_maps) } pub fn filemap_by_stable_id(&self, stable_id: StableFilemapId) -> Option> { - 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. @@ -206,6 +213,7 @@ 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 { let start_pos = self.next_start_pos(); @@ -231,16 +239,16 @@ impl CodeMap { Pos::from_usize(start_pos), )); - self.files.borrow_mut().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 { let fm = self.new_filemap(filename.to_owned().into(), src.to_owned()); let mut byte_pos: u32 = fm.start_pos.0; @@ -303,11 +311,10 @@ impl CodeMap { name_hash, }); - self.files.borrow_mut().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 } @@ -398,7 +405,7 @@ impl CodeMap { pub fn lookup_line(&self, pos: BytePos) -> Result> { let idx = self.lookup_filemap_idx(pos); - let f = (*self.files.borrow())[idx].clone(); + let f = (*self.files.borrow().file_maps)[idx].clone(); match f.lookup_line(pos) { Some(line) => Ok(FileMapAndLine { fm: f, line: line }), @@ -452,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(); } @@ -787,7 +794,7 @@ impl CodeMap { } pub fn get_filemap(&self, filename: &FileName) -> Option> { - for fm in self.files.borrow().iter() { + for fm in self.files.borrow().file_maps.iter() { if *filename == fm.name { return Some(fm.clone()); } @@ -798,7 +805,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} } @@ -806,7 +813,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 map = &(*self.files.borrow())[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; @@ -832,7 +839,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/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) { -- cgit 1.4.1-3-g733a5