Skip to content

Commit f8e4afc

Browse files
ZoxcZoxc
committed
Use Freeze for SourceFile.external_src 
1 parent f00c139 commit f8e4afc

File tree

8 files changed

+91
-47
lines changed

8 files changed

+91
-47
lines changed

‎compiler/rustc_data_structures/src/sync/freeze.rs

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,26 @@ unsafe impl<T: DynSync + DynSend> DynSync for FreezeLock<T> {}
2727
impl<T> FreezeLock<T> {
2828
#[inline]
2929
pub fn new(value: T) -> Self {
30-
Self { data: UnsafeCell::new(value), frozen: AtomicBool::new(false), lock: RwLock::new(()) }
30+
Self::with(value, false)
31+
}
32+
33+
#[inline]
34+
pub fn frozen(value: T) -> Self {
35+
Self::with(value, true)
36+
}
37+
38+
#[inline]
39+
pub fn with(value: T, frozen: bool) -> Self {
40+
Self {
41+
data: UnsafeCell::new(value),
42+
frozen: AtomicBool::new(frozen),
43+
lock: RwLock::new(()),
44+
}
45+
}
46+
47+
#[inline]
48+
pub fn is_frozen(&self) -> bool {
49+
self.frozen.load(Ordering::Acquire)
3150
}
3251

3352
#[inline]
@@ -42,13 +61,25 @@ impl<T> FreezeLock<T> {
4261
}
4362
}
4463

64+
#[inline]
65+
pub fn borrow(&self) -> FreezeReadGuard<'_, T> {
66+
self.read()
67+
}
68+
4569
#[inline]
4670
#[track_caller]
4771
pub fn write(&self) -> FreezeWriteGuard<'_, T> {
72+
self.try_write().expect("still mutable")
73+
}
74+
75+
#[inline]
76+
pub fn try_write(&self) -> Option<FreezeWriteGuard<'_, T>> {
4877
let _lock_guard = self.lock.write();
49-
// Use relaxed ordering since we're in the write lock.
50-
assert!(!self.frozen.load(Ordering::Relaxed), "still mutable");
51-
FreezeWriteGuard { _lock_guard, lock: self, marker: PhantomData }
78+
if self.frozen.load(Ordering::Relaxed) {
79+
None
80+
} else {
81+
Some(FreezeWriteGuard { _lock_guard, lock: self, marker: PhantomData })
82+
}
5283
}
5384

5485
#[inline]
@@ -90,6 +121,15 @@ pub struct FreezeWriteGuard<'a, T> {
90121
marker: PhantomData<&'a mut T>,
91122
}
92123

124+
impl<'a, T> FreezeWriteGuard<'a, T> {
125+
pub fn freeze(self) -> &'a T {
126+
self.lock.frozen.store(true, Ordering::Release);
127+
128+
// SAFETY: This is frozen so the data cannot be modified and shared access is sound.
129+
unsafe { &*self.lock.data.get() }
130+
}
131+
}
132+
93133
impl<'a, T: 'a> Deref for FreezeWriteGuard<'a, T> {
94134
type Target = T;
95135
#[inline]

‎compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ impl AnnotateSnippetEmitterWriter {
169169
.map(|line| {
170170
// Ensure the source file is present before we try
171171
// to load a string from it.
172-
source_map.ensure_source_file_source_present(file.clone());
172+
source_map.ensure_source_file_source_present(&file);
173173
(
174174
format!("{}", source_map.filename_for_diagnostics(&file.name)),
175175
source_string(file.clone(), &line),

‎compiler/rustc_errors/src/emitter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,7 +1193,7 @@ impl EmitterWriter {
11931193
let will_be_emitted = |span: Span| {
11941194
!span.is_dummy() && {
11951195
let file = sm.lookup_source_file(span.hi());
1196-
sm.ensure_source_file_source_present(file)
1196+
sm.ensure_source_file_source_present(&file)
11971197
}
11981198
};
11991199

@@ -1388,7 +1388,7 @@ impl EmitterWriter {
13881388
// Print out the annotate source lines that correspond with the error
13891389
for annotated_file in annotated_files {
13901390
// we can't annotate anything if the source is unavailable.
1391-
if !sm.ensure_source_file_source_present(annotated_file.file.clone()) {
1391+
if !sm.ensure_source_file_source_present(&annotated_file.file) {
13921392
if !self.short_message {
13931393
// We'll just print an unannotated message.
13941394
for (annotation_id, line) in annotated_file.lines.iter().enumerate() {

‎compiler/rustc_errors/src/json.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ impl DiagnosticSpanLine {
558558
.span_to_lines(span)
559559
.map(|lines| {
560560
// We can't get any lines if the source is unavailable.
561-
if !je.sm.ensure_source_file_source_present(lines.file.clone()) {
561+
if !je.sm.ensure_source_file_source_present(&lines.file) {
562562
return vec![];
563563
}
564564

‎compiler/rustc_errors/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ impl CodeSuggestion {
273273
assert!(!lines.lines.is_empty() || bounding_span.is_dummy());
274274

275275
// We can't splice anything if the source is unavailable.
276-
if !sm.ensure_source_file_source_present(lines.file.clone()) {
276+
if !sm.ensure_source_file_source_present(&lines.file) {
277277
return None;
278278
}
279279

‎compiler/rustc_metadata/src/rmeta/encoder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,8 @@ impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for SpanData {
280280
// All of this logic ensures that the final result of deserialization is a 'normal'
281281
// Span that can be used without any additional trouble.
282282
let metadata_index = {
283-
// Introduce a new scope so that we drop the 'lock()' temporary
284-
match &*source_file.external_src.lock() {
283+
// Introduce a new scope so that we drop the 'read()' temporary
284+
match &*source_file.external_src.read() {
285285
ExternalSource::Foreign { metadata_index, .. } => *metadata_index,
286286
src => panic!("Unexpected external source {src:?}"),
287287
}

‎compiler/rustc_span/src/lib.rs

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ pub mod fatal_error;
6464
pub mod profiling;
6565

6666
use rustc_data_structures::stable_hasher::{Hash128, Hash64, HashStable, StableHasher};
67-
use rustc_data_structures::sync::{Lock, Lrc};
67+
use rustc_data_structures::sync::{FreezeLock, FreezeWriteGuard, Lock, Lrc};
6868

6969
use std::borrow::Cow;
7070
use std::cmp::{self, Ordering};
@@ -1206,7 +1206,6 @@ pub enum ExternalSourceKind {
12061206
AbsentOk,
12071207
/// A failed attempt has been made to load the external source.
12081208
AbsentErr,
1209-
Unneeded,
12101209
}
12111210

12121211
impl ExternalSource {
@@ -1343,7 +1342,7 @@ pub struct SourceFile {
13431342
pub src_hash: SourceFileHash,
13441343
/// The external source code (used for external crates, which will have a `None`
13451344
/// value as `self.src`.
1346-
pub external_src: Lock<ExternalSource>,
1345+
pub external_src: FreezeLock<ExternalSource>,
13471346
/// The start position of this source in the `SourceMap`.
13481347
pub start_pos: BytePos,
13491348
/// The byte length of this source.
@@ -1368,7 +1367,10 @@ impl Clone for SourceFile {
13681367
name: self.name.clone(),
13691368
src: self.src.clone(),
13701369
src_hash: self.src_hash,
1371-
external_src: Lock::new(self.external_src.borrow().clone()),
1370+
external_src: {
1371+
let lock = self.external_src.read();
1372+
FreezeLock::with(lock.clone(), self.external_src.is_frozen())
1373+
},
13721374
start_pos: self.start_pos,
13731375
source_len: self.source_len,
13741376
lines: Lock::new(self.lines.borrow().clone()),
@@ -1488,7 +1490,7 @@ impl<D: Decoder> Decodable<D> for SourceFile {
14881490
src_hash,
14891491
// Unused - the metadata decoder will construct
14901492
// a new SourceFile, filling in `external_src` properly
1491-
external_src: Lock::new(ExternalSource::Unneeded),
1493+
external_src: FreezeLock::frozen(ExternalSource::Unneeded),
14921494
lines: Lock::new(lines),
14931495
multibyte_chars,
14941496
non_narrow_chars,
@@ -1530,7 +1532,7 @@ impl SourceFile {
15301532
name,
15311533
src: Some(Lrc::new(src)),
15321534
src_hash,
1533-
external_src: Lock::new(ExternalSource::Unneeded),
1535+
external_src: FreezeLock::frozen(ExternalSource::Unneeded),
15341536
start_pos: BytePos::from_u32(0),
15351537
source_len: RelativeBytePos::from_u32(source_len),
15361538
lines: Lock::new(SourceFileLines::Lines(lines)),
@@ -1612,35 +1614,37 @@ impl SourceFile {
16121614
where
16131615
F: FnOnce() -> Option<String>,
16141616
{
1615-
if matches!(
1616-
*self.external_src.borrow(),
1617-
ExternalSource::Foreign { kind: ExternalSourceKind::AbsentOk, .. }
1618-
) {
1617+
if !self.external_src.is_frozen() {
16191618
let src = get_src();
1620-
let mut external_src = self.external_src.borrow_mut();
1621-
// Check that no-one else have provided the source while we were getting it
1622-
if let ExternalSource::Foreign {
1623-
kind: src_kind @ ExternalSourceKind::AbsentOk, ..
1624-
} = &mut *external_src
1625-
{
1626-
if let Some(mut src) = src {
1627-
// The src_hash needs to be computed on the pre-normalized src.
1628-
if self.src_hash.matches(&src) {
1629-
normalize_src(&mut src);
1630-
*src_kind = ExternalSourceKind::Present(Lrc::new(src));
1631-
return true;
1632-
}
1619+
let src = src.and_then(|mut src| {
1620+
// The src_hash needs to be computed on the pre-normalized src.
1621+
self.src_hash.matches(&src).then(|| {
1622+
normalize_src(&mut src);
1623+
src
1624+
})
1625+
});
1626+
1627+
self.external_src.try_write().map(|mut external_src| {
1628+
if let ExternalSource::Foreign {
1629+
kind: src_kind @ ExternalSourceKind::AbsentOk,
1630+
..
1631+
} = &mut *external_src
1632+
{
1633+
*src_kind = if let Some(src) = src {
1634+
ExternalSourceKind::Present(Lrc::new(src))
1635+
} else {
1636+
ExternalSourceKind::AbsentErr
1637+
};
16331638
} else {
1634-
*src_kind = ExternalSourceKind::AbsentErr;
1639+
panic!("unexpected state {:?}", *external_src)
16351640
}
16361641

1637-
false
1638-
} else {
1639-
self.src.is_some() || external_src.get_source().is_some()
1640-
}
1641-
} else {
1642-
self.src.is_some() || self.external_src.borrow().get_source().is_some()
1642+
// Freeze this so we don't try to load the source again.
1643+
FreezeWriteGuard::freeze(external_src)
1644+
});
16431645
}
1646+
1647+
self.src.is_some() || self.external_src.read().get_source().is_some()
16441648
}
16451649

16461650
/// Gets a line from the list of pre-computed line-beginnings.

‎compiler/rustc_span/src/source_map.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ impl SourceMap {
340340
name: filename,
341341
src: None,
342342
src_hash,
343-
external_src: Lock::new(ExternalSource::Foreign {
343+
external_src: FreezeLock::new(ExternalSource::Foreign {
344344
kind: ExternalSourceKind::AbsentOk,
345345
metadata_index,
346346
}),
@@ -564,7 +564,7 @@ impl SourceMap {
564564
end: (local_end.sf.name.clone(), local_end.sf.start_pos),
565565
})))
566566
} else {
567-
self.ensure_source_file_source_present(local_begin.sf.clone());
567+
self.ensure_source_file_source_present(&local_begin.sf);
568568

569569
let start_index = local_begin.pos.to_usize();
570570
let end_index = local_end.pos.to_usize();
@@ -581,7 +581,7 @@ impl SourceMap {
581581

582582
if let Some(ref src) = local_begin.sf.src {
583583
extract_source(src, start_index, end_index)
584-
} else if let Some(src) = local_begin.sf.external_src.borrow().get_source() {
584+
} else if let Some(src) = local_begin.sf.external_src.read().get_source() {
585585
extract_source(src, start_index, end_index)
586586
} else {
587587
Err(SpanSnippetError::SourceNotAvailable { filename: local_begin.sf.name.clone() })
@@ -873,7 +873,7 @@ impl SourceMap {
873873
let sp = sp.data();
874874
let local_begin = self.lookup_byte_offset(sp.lo);
875875
let start_index = local_begin.pos.to_usize();
876-
let src = local_begin.sf.external_src.borrow();
876+
let src = local_begin.sf.external_src.read();
877877

878878
let snippet = if let Some(ref src) = local_begin.sf.src {
879879
Some(&src[start_index..])
@@ -983,7 +983,7 @@ impl SourceMap {
983983
return 1;
984984
}
985985

986-
let src = local_begin.sf.external_src.borrow();
986+
let src = local_begin.sf.external_src.read();
987987

988988
let snippet = if let Some(src) = &local_begin.sf.src {
989989
src
@@ -1030,7 +1030,7 @@ impl SourceMap {
10301030
self.files().iter().fold(0, |a, f| a + f.count_lines())
10311031
}
10321032

1033-
pub fn ensure_source_file_source_present(&self, source_file: Lrc<SourceFile>) -> bool {
1033+
pub fn ensure_source_file_source_present(&self, source_file: &SourceFile) -> bool {
10341034
source_file.add_external_src(|| {
10351035
let FileName::Real(ref name) = source_file.name else {
10361036
return None;

0 commit comments

Comments
 (0)