Skip to content

Commit 7ec7a92

Browse files
authored
Rollup merge of rust-lang#149989 - Urgau:filenames-post-improvements, r=davidtwco
Improve filenames encoding and misc This PR is a follow-up to rust-lang#149709, it aims at preventing a double encoding when there are no remapping, as well as making some small improvements to the code. Best reviewed commit by commit.
2 parents 508c382 + 075f4cd commit 7ec7a92

File tree

1 file changed

+51
-5
lines changed

1 file changed

+51
-5
lines changed

compiler/rustc_span/src/lib.rs

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,14 @@ bitflags::bitflags! {
241241
}
242242

243243
impl<E: Encoder> Encodable<E> for RemapPathScopeComponents {
244+
#[inline]
244245
fn encode(&self, s: &mut E) {
245246
s.emit_u8(self.bits());
246247
}
247248
}
248249

249250
impl<D: Decoder> Decodable<D> for RemapPathScopeComponents {
251+
#[inline]
250252
fn decode(s: &mut D) -> RemapPathScopeComponents {
251253
RemapPathScopeComponents::from_bits(s.read_u8())
252254
.expect("invalid bits for RemapPathScopeComponents")
@@ -308,12 +310,13 @@ struct InnerRealFileName {
308310
}
309311

310312
impl Hash for RealFileName {
313+
#[inline]
311314
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
312315
// To prevent #70924 from happening again we should only hash the
313316
// remapped path if that exists. This is because remapped paths to
314317
// sysroot crates (/rust/$hash or /rust/$version) remain stable even
315318
// if the corresponding local path changes.
316-
if !self.scopes.is_all() {
319+
if !self.was_fully_remapped() {
317320
self.local.hash(state);
318321
}
319322
self.maybe_remapped.hash(state);
@@ -327,6 +330,7 @@ impl RealFileName {
327330
/// ## Panic
328331
///
329332
/// Only one scope components can be given to this function.
333+
#[inline]
330334
pub fn path(&self, scope: RemapPathScopeComponents) -> &Path {
331335
assert!(
332336
scope.bits().count_ones() == 1,
@@ -351,6 +355,7 @@ impl RealFileName {
351355
/// ## Panic
352356
///
353357
/// Only one scope components can be given to this function.
358+
#[inline]
354359
pub fn embeddable_name(&self, scope: RemapPathScopeComponents) -> (&Path, &Path) {
355360
assert!(
356361
scope.bits().count_ones() == 1,
@@ -369,26 +374,58 @@ impl RealFileName {
369374
/// if this information exists.
370375
///
371376
/// May not exists if the filename was imported from another crate.
377+
///
378+
/// Avoid embedding this in build artifacts; prefer `path()` or `embeddable_name()`.
379+
#[inline]
372380
pub fn local_path(&self) -> Option<&Path> {
373-
self.local.as_ref().map(|lp| lp.name.as_ref())
381+
if self.was_not_remapped() {
382+
Some(&self.maybe_remapped.name)
383+
} else if let Some(local) = &self.local {
384+
Some(&local.name)
385+
} else {
386+
None
387+
}
374388
}
375389

376390
/// Returns the path suitable for reading from the file system on the local host,
377391
/// if this information exists.
378392
///
379393
/// May not exists if the filename was imported from another crate.
394+
///
395+
/// Avoid embedding this in build artifacts; prefer `path()` or `embeddable_name()`.
396+
#[inline]
380397
pub fn into_local_path(self) -> Option<PathBuf> {
381-
self.local.map(|lp| lp.name)
398+
if self.was_not_remapped() {
399+
Some(self.maybe_remapped.name)
400+
} else if let Some(local) = self.local {
401+
Some(local.name)
402+
} else {
403+
None
404+
}
382405
}
383406

384407
/// Returns whenever the filename was remapped.
408+
#[inline]
385409
pub(crate) fn was_remapped(&self) -> bool {
386410
!self.scopes.is_empty()
387411
}
388412

413+
/// Returns whenever the filename was fully remapped.
414+
#[inline]
415+
fn was_fully_remapped(&self) -> bool {
416+
self.scopes.is_all()
417+
}
418+
419+
/// Returns whenever the filename was not remapped.
420+
#[inline]
421+
fn was_not_remapped(&self) -> bool {
422+
self.scopes.is_empty()
423+
}
424+
389425
/// Returns an empty `RealFileName`
390426
///
391427
/// Useful as the working directory input to `SourceMap::to_real_filename`.
428+
#[inline]
392429
pub fn empty() -> RealFileName {
393430
RealFileName {
394431
local: Some(InnerRealFileName {
@@ -420,9 +457,14 @@ impl RealFileName {
420457
/// Update the filename for encoding in the crate metadata.
421458
///
422459
/// Currently it's about removing the local part when the filename
423-
/// is fully remapped.
460+
/// is either fully remapped or not remapped at all.
461+
#[inline]
424462
pub fn update_for_crate_metadata(&mut self) {
425-
if self.scopes.is_all() {
463+
if self.was_fully_remapped() || self.was_not_remapped() {
464+
// NOTE: This works because when the filename is fully
465+
// remapped, we don't care about the `local` part,
466+
// and when the filename is not remapped at all,
467+
// `maybe_remapped` and `local` are equal.
426468
self.local = None;
427469
}
428470
}
@@ -529,6 +571,7 @@ impl FileName {
529571
/// if this information exists.
530572
///
531573
/// Avoid embedding this in build artifacts. Prefer using the `display` method.
574+
#[inline]
532575
pub fn prefer_remapped_unconditionally(&self) -> FileNameDisplay<'_> {
533576
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Remapped }
534577
}
@@ -537,16 +580,19 @@ impl FileName {
537580
/// if this information exists.
538581
///
539582
/// Avoid embedding this in build artifacts. Prefer using the `display` method.
583+
#[inline]
540584
pub fn prefer_local_unconditionally(&self) -> FileNameDisplay<'_> {
541585
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Local }
542586
}
543587

544588
/// Returns a short (either the filename or an empty string).
589+
#[inline]
545590
pub fn short(&self) -> FileNameDisplay<'_> {
546591
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Short }
547592
}
548593

549594
/// Returns a `Display`-able path for the given scope.
595+
#[inline]
550596
pub fn display(&self, scope: RemapPathScopeComponents) -> FileNameDisplay<'_> {
551597
FileNameDisplay { inner: self, display_pref: FileNameDisplayPreference::Scope(scope) }
552598
}

0 commit comments

Comments
 (0)