Use Freeze for SourceFile.external_src

This commit is contained in:
John Kåre Alsaker 2023-08-31 12:50:44 +02:00
parent f00c139998
commit c5996b80be
8 changed files with 91 additions and 46 deletions

View file

@ -27,7 +27,26 @@ unsafe impl<T: DynSync + DynSend> DynSync for FreezeLock<T> {}
impl<T> FreezeLock<T> { impl<T> FreezeLock<T> {
#[inline] #[inline]
pub fn new(value: T) -> Self { pub fn new(value: T) -> Self {
Self { data: UnsafeCell::new(value), frozen: AtomicBool::new(false), lock: RwLock::new(()) } Self::with(value, false)
}
#[inline]
pub fn frozen(value: T) -> Self {
Self::with(value, true)
}
#[inline]
pub fn with(value: T, frozen: bool) -> Self {
Self {
data: UnsafeCell::new(value),
frozen: AtomicBool::new(frozen),
lock: RwLock::new(()),
}
}
#[inline]
pub fn is_frozen(&self) -> bool {
self.frozen.load(Ordering::Acquire)
} }
#[inline] #[inline]
@ -42,13 +61,26 @@ impl<T> FreezeLock<T> {
} }
} }
#[inline]
pub fn borrow(&self) -> FreezeReadGuard<'_, T> {
self.read()
}
#[inline] #[inline]
#[track_caller] #[track_caller]
pub fn write(&self) -> FreezeWriteGuard<'_, T> { pub fn write(&self) -> FreezeWriteGuard<'_, T> {
self.try_write().expect("still mutable")
}
#[inline]
pub fn try_write(&self) -> Option<FreezeWriteGuard<'_, T>> {
let _lock_guard = self.lock.write(); let _lock_guard = self.lock.write();
// Use relaxed ordering since we're in the write lock. // Use relaxed ordering since we're in the write lock.
assert!(!self.frozen.load(Ordering::Relaxed), "still mutable"); if self.frozen.load(Ordering::Relaxed) {
FreezeWriteGuard { _lock_guard, lock: self, marker: PhantomData } None
} else {
Some(FreezeWriteGuard { _lock_guard, lock: self, marker: PhantomData })
}
} }
#[inline] #[inline]
@ -90,6 +122,15 @@ pub struct FreezeWriteGuard<'a, T> {
marker: PhantomData<&'a mut T>, marker: PhantomData<&'a mut T>,
} }
impl<'a, T> FreezeWriteGuard<'a, T> {
pub fn freeze(self) -> &'a T {
self.lock.frozen.store(true, Ordering::Release);
// SAFETY: This is frozen so the data cannot be modified and shared access is sound.
unsafe { &*self.lock.data.get() }
}
}
impl<'a, T: 'a> Deref for FreezeWriteGuard<'a, T> { impl<'a, T: 'a> Deref for FreezeWriteGuard<'a, T> {
type Target = T; type Target = T;
#[inline] #[inline]

View file

@ -169,7 +169,7 @@ impl AnnotateSnippetEmitterWriter {
.map(|line| { .map(|line| {
// Ensure the source file is present before we try // Ensure the source file is present before we try
// to load a string from it. // to load a string from it.
source_map.ensure_source_file_source_present(file.clone()); source_map.ensure_source_file_source_present(&file);
( (
format!("{}", source_map.filename_for_diagnostics(&file.name)), format!("{}", source_map.filename_for_diagnostics(&file.name)),
source_string(file.clone(), &line), source_string(file.clone(), &line),

View file

@ -1193,7 +1193,7 @@ impl EmitterWriter {
let will_be_emitted = |span: Span| { let will_be_emitted = |span: Span| {
!span.is_dummy() && { !span.is_dummy() && {
let file = sm.lookup_source_file(span.hi()); let file = sm.lookup_source_file(span.hi());
sm.ensure_source_file_source_present(file) sm.ensure_source_file_source_present(&file)
} }
}; };
@ -1388,7 +1388,7 @@ impl EmitterWriter {
// Print out the annotate source lines that correspond with the error // Print out the annotate source lines that correspond with the error
for annotated_file in annotated_files { for annotated_file in annotated_files {
// we can't annotate anything if the source is unavailable. // we can't annotate anything if the source is unavailable.
if !sm.ensure_source_file_source_present(annotated_file.file.clone()) { if !sm.ensure_source_file_source_present(&annotated_file.file) {
if !self.short_message { if !self.short_message {
// We'll just print an unannotated message. // We'll just print an unannotated message.
for (annotation_id, line) in annotated_file.lines.iter().enumerate() { for (annotation_id, line) in annotated_file.lines.iter().enumerate() {

View file

@ -558,7 +558,7 @@ impl DiagnosticSpanLine {
.span_to_lines(span) .span_to_lines(span)
.map(|lines| { .map(|lines| {
// We can't get any lines if the source is unavailable. // We can't get any lines if the source is unavailable.
if !je.sm.ensure_source_file_source_present(lines.file.clone()) { if !je.sm.ensure_source_file_source_present(&lines.file) {
return vec![]; return vec![];
} }

View file

@ -273,7 +273,7 @@ impl CodeSuggestion {
assert!(!lines.lines.is_empty() || bounding_span.is_dummy()); assert!(!lines.lines.is_empty() || bounding_span.is_dummy());
// We can't splice anything if the source is unavailable. // We can't splice anything if the source is unavailable.
if !sm.ensure_source_file_source_present(lines.file.clone()) { if !sm.ensure_source_file_source_present(&lines.file) {
return None; return None;
} }

View file

@ -280,8 +280,8 @@ impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for SpanData {
// All of this logic ensures that the final result of deserialization is a 'normal' // All of this logic ensures that the final result of deserialization is a 'normal'
// Span that can be used without any additional trouble. // Span that can be used without any additional trouble.
let metadata_index = { let metadata_index = {
// Introduce a new scope so that we drop the 'lock()' temporary // Introduce a new scope so that we drop the 'read()' temporary
match &*source_file.external_src.lock() { match &*source_file.external_src.read() {
ExternalSource::Foreign { metadata_index, .. } => *metadata_index, ExternalSource::Foreign { metadata_index, .. } => *metadata_index,
src => panic!("Unexpected external source {src:?}"), src => panic!("Unexpected external source {src:?}"),
} }

View file

@ -64,7 +64,7 @@ pub mod fatal_error;
pub mod profiling; pub mod profiling;
use rustc_data_structures::stable_hasher::{Hash128, Hash64, HashStable, StableHasher}; use rustc_data_structures::stable_hasher::{Hash128, Hash64, HashStable, StableHasher};
use rustc_data_structures::sync::{Lock, Lrc}; use rustc_data_structures::sync::{FreezeLock, FreezeWriteGuard, Lock, Lrc};
use std::borrow::Cow; use std::borrow::Cow;
use std::cmp::{self, Ordering}; use std::cmp::{self, Ordering};
@ -1206,7 +1206,6 @@ pub enum ExternalSourceKind {
AbsentOk, AbsentOk,
/// A failed attempt has been made to load the external source. /// A failed attempt has been made to load the external source.
AbsentErr, AbsentErr,
Unneeded,
} }
impl ExternalSource { impl ExternalSource {
@ -1343,7 +1342,7 @@ pub struct SourceFile {
pub src_hash: SourceFileHash, pub src_hash: SourceFileHash,
/// The external source code (used for external crates, which will have a `None` /// The external source code (used for external crates, which will have a `None`
/// value as `self.src`. /// value as `self.src`.
pub external_src: Lock<ExternalSource>, pub external_src: FreezeLock<ExternalSource>,
/// The start position of this source in the `SourceMap`. /// The start position of this source in the `SourceMap`.
pub start_pos: BytePos, pub start_pos: BytePos,
/// The byte length of this source. /// The byte length of this source.
@ -1368,7 +1367,10 @@ impl Clone for SourceFile {
name: self.name.clone(), name: self.name.clone(),
src: self.src.clone(), src: self.src.clone(),
src_hash: self.src_hash, src_hash: self.src_hash,
external_src: Lock::new(self.external_src.borrow().clone()), external_src: {
let lock = self.external_src.read();
FreezeLock::with(lock.clone(), self.external_src.is_frozen())
},
start_pos: self.start_pos, start_pos: self.start_pos,
source_len: self.source_len, source_len: self.source_len,
lines: Lock::new(self.lines.borrow().clone()), lines: Lock::new(self.lines.borrow().clone()),
@ -1488,7 +1490,7 @@ impl<D: Decoder> Decodable<D> for SourceFile {
src_hash, src_hash,
// Unused - the metadata decoder will construct // Unused - the metadata decoder will construct
// a new SourceFile, filling in `external_src` properly // a new SourceFile, filling in `external_src` properly
external_src: Lock::new(ExternalSource::Unneeded), external_src: FreezeLock::frozen(ExternalSource::Unneeded),
lines: Lock::new(lines), lines: Lock::new(lines),
multibyte_chars, multibyte_chars,
non_narrow_chars, non_narrow_chars,
@ -1530,7 +1532,7 @@ impl SourceFile {
name, name,
src: Some(Lrc::new(src)), src: Some(Lrc::new(src)),
src_hash, src_hash,
external_src: Lock::new(ExternalSource::Unneeded), external_src: FreezeLock::frozen(ExternalSource::Unneeded),
start_pos: BytePos::from_u32(0), start_pos: BytePos::from_u32(0),
source_len: RelativeBytePos::from_u32(source_len), source_len: RelativeBytePos::from_u32(source_len),
lines: Lock::new(SourceFileLines::Lines(lines)), lines: Lock::new(SourceFileLines::Lines(lines)),
@ -1612,35 +1614,37 @@ impl SourceFile {
where where
F: FnOnce() -> Option<String>, F: FnOnce() -> Option<String>,
{ {
if matches!( if !self.external_src.is_frozen() {
*self.external_src.borrow(),
ExternalSource::Foreign { kind: ExternalSourceKind::AbsentOk, .. }
) {
let src = get_src(); let src = get_src();
let mut external_src = self.external_src.borrow_mut(); let src = src.and_then(|mut src| {
// Check that no-one else have provided the source while we were getting it // The src_hash needs to be computed on the pre-normalized src.
if let ExternalSource::Foreign { self.src_hash.matches(&src).then(|| {
kind: src_kind @ ExternalSourceKind::AbsentOk, .. normalize_src(&mut src);
} = &mut *external_src src
{ })
if let Some(mut src) = src { });
// The src_hash needs to be computed on the pre-normalized src.
if self.src_hash.matches(&src) { self.external_src.try_write().map(|mut external_src| {
normalize_src(&mut src); if let ExternalSource::Foreign {
*src_kind = ExternalSourceKind::Present(Lrc::new(src)); kind: src_kind @ ExternalSourceKind::AbsentOk,
return true; ..
} } = &mut *external_src
{
*src_kind = if let Some(src) = src {
ExternalSourceKind::Present(Lrc::new(src))
} else {
ExternalSourceKind::AbsentErr
};
} else { } else {
*src_kind = ExternalSourceKind::AbsentErr; panic!("unexpected state {:?}", *external_src)
} }
false // Freeze this so we don't try to load the source again.
} else { FreezeWriteGuard::freeze(external_src)
self.src.is_some() || external_src.get_source().is_some() });
}
} else {
self.src.is_some() || self.external_src.borrow().get_source().is_some()
} }
self.src.is_some() || self.external_src.read().get_source().is_some()
} }
/// Gets a line from the list of pre-computed line-beginnings. /// Gets a line from the list of pre-computed line-beginnings.

View file

@ -340,7 +340,7 @@ impl SourceMap {
name: filename, name: filename,
src: None, src: None,
src_hash, src_hash,
external_src: Lock::new(ExternalSource::Foreign { external_src: FreezeLock::new(ExternalSource::Foreign {
kind: ExternalSourceKind::AbsentOk, kind: ExternalSourceKind::AbsentOk,
metadata_index, metadata_index,
}), }),
@ -564,7 +564,7 @@ impl SourceMap {
end: (local_end.sf.name.clone(), local_end.sf.start_pos), end: (local_end.sf.name.clone(), local_end.sf.start_pos),
}))) })))
} else { } else {
self.ensure_source_file_source_present(local_begin.sf.clone()); self.ensure_source_file_source_present(&local_begin.sf);
let start_index = local_begin.pos.to_usize(); let start_index = local_begin.pos.to_usize();
let end_index = local_end.pos.to_usize(); let end_index = local_end.pos.to_usize();
@ -581,7 +581,7 @@ impl SourceMap {
if let Some(ref src) = local_begin.sf.src { if let Some(ref src) = local_begin.sf.src {
extract_source(src, start_index, end_index) extract_source(src, start_index, end_index)
} else if let Some(src) = local_begin.sf.external_src.borrow().get_source() { } else if let Some(src) = local_begin.sf.external_src.read().get_source() {
extract_source(src, start_index, end_index) extract_source(src, start_index, end_index)
} else { } else {
Err(SpanSnippetError::SourceNotAvailable { filename: local_begin.sf.name.clone() }) Err(SpanSnippetError::SourceNotAvailable { filename: local_begin.sf.name.clone() })
@ -873,7 +873,7 @@ impl SourceMap {
let sp = sp.data(); let sp = sp.data();
let local_begin = self.lookup_byte_offset(sp.lo); let local_begin = self.lookup_byte_offset(sp.lo);
let start_index = local_begin.pos.to_usize(); let start_index = local_begin.pos.to_usize();
let src = local_begin.sf.external_src.borrow(); let src = local_begin.sf.external_src.read();
let snippet = if let Some(ref src) = local_begin.sf.src { let snippet = if let Some(ref src) = local_begin.sf.src {
Some(&src[start_index..]) Some(&src[start_index..])
@ -983,7 +983,7 @@ impl SourceMap {
return 1; return 1;
} }
let src = local_begin.sf.external_src.borrow(); let src = local_begin.sf.external_src.read();
let snippet = if let Some(src) = &local_begin.sf.src { let snippet = if let Some(src) = &local_begin.sf.src {
src src
@ -1030,7 +1030,7 @@ impl SourceMap {
self.files().iter().fold(0, |a, f| a + f.count_lines()) self.files().iter().fold(0, |a, f| a + f.count_lines())
} }
pub fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool { pub fn ensure_source_file_source_present(&self, source_file: &SourceFile) -> bool {
source_file.add_external_src(|| { source_file.add_external_src(|| {
let FileName::Real(ref name) = source_file.name else { let FileName::Real(ref name) = source_file.name else {
return None; return None;