Auto merge of #117507 - nnethercote:rustc_span, r=Nilstrieb

`rustc_span` cleanups

Just some things I found while looking over this crate.

r? `@oli-obk`

5 files changed, 46 insertions, 106 deletions

diff --git a/compiler/rustc_span/src/fatal_error.rs b/compiler/rustc_span/src/fatal_error.rs
index fa84c486df5..8b4f77a0566 100644
--- a/compiler/rustc_span/src/fatal_error.rs
+++ b/compiler/rustc_span/src/fatal_error.rs
@@ -1,13 +1,11 @@
-/// Used as a return value to signify a fatal error occurred. (It is also
-/// used as the argument to panic at the moment, but that will eventually
-/// not be true.)
+/// Used as a return value to signify a fatal error occurred.
 #[derive(Copy, Clone, Debug)]
 #[must_use]
 pub struct FatalError;
 
 pub struct FatalErrorMarker;
 
-// Don't implement Send on FatalError. This makes it impossible to panic!(FatalError).
+// Don't implement Send on FatalError. This makes it impossible to `panic_any!(FatalError)`.
 // We don't want to invoke the panic handler and print a backtrace for fatal errors.
 impl !Send for FatalError {}
 
diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs
index 88081700c3b..36731d0fe74 100644
--- a/compiler/rustc_span/src/hygiene.rs
+++ b/compiler/rustc_span/src/hygiene.rs
@@ -24,16 +24,13 @@
 // because getting it wrong can lead to nested `HygieneData::with` calls that
 // trigger runtime aborts. (Fortunately these are obvious and easy to fix.)
 
+use crate::def_id::{CrateNum, DefId, StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
 use crate::edition::Edition;
 use crate::symbol::{kw, sym, Symbol};
-use crate::with_session_globals;
-use crate::{HashStableContext, Span, DUMMY_SP};
-
-use crate::def_id::{CrateNum, DefId, StableCrateId, CRATE_DEF_ID, LOCAL_CRATE};
+use crate::{with_session_globals, HashStableContext, Span, DUMMY_SP};
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_data_structures::stable_hasher::HashingControls;
-use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
+use rustc_data_structures::stable_hasher::{Hash64, HashStable, HashingControls, StableHasher};
 use rustc_data_structures::sync::{Lock, Lrc, WorkerLocal};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_index::IndexVec;
@@ -130,7 +127,7 @@ impl ExpnHash {
 
     /// Returns the crate-local part of the [ExpnHash].
     ///
-    /// Used for tests.
+    /// Used for assertions.
     #[inline]
     pub fn local_hash(self) -> Hash64 {
         self.0.split().1
@@ -173,7 +170,7 @@ impl LocalExpnId {
     pub const ROOT: LocalExpnId = LocalExpnId::from_u32(0);
 
     #[inline]
-    pub fn from_raw(idx: ExpnIndex) -> LocalExpnId {
+    fn from_raw(idx: ExpnIndex) -> LocalExpnId {
         LocalExpnId::from_u32(idx.as_u32())
     }
 
@@ -205,11 +202,6 @@ impl LocalExpnId {
     }
 
     #[inline]
-    pub fn expn_hash(self) -> ExpnHash {
-        HygieneData::with(|data| data.local_expn_hash(self))
-    }
-
-    #[inline]
     pub fn expn_data(self) -> ExpnData {
         HygieneData::with(|data| data.local_expn_data(self).clone())
     }
@@ -239,13 +231,6 @@ impl LocalExpnId {
         self.to_expn_id().is_descendant_of(ancestor.to_expn_id())
     }
 
-    /// `expn_id.outer_expn_is_descendant_of(ctxt)` is equivalent to but faster than
-    /// `expn_id.is_descendant_of(ctxt.outer_expn())`.
-    #[inline]
-    pub fn outer_expn_is_descendant_of(self, ctxt: SyntaxContext) -> bool {
-        self.to_expn_id().outer_expn_is_descendant_of(ctxt)
-    }
-
     /// Returns span for the macro which originally caused this expansion to happen.
     ///
     /// Stops backtracing at include! boundary.
@@ -253,12 +238,6 @@ impl LocalExpnId {
     pub fn expansion_cause(self) -> Option<Span> {
         self.to_expn_id().expansion_cause()
     }
-
-    #[inline]
-    #[track_caller]
-    pub fn parent(self) -> LocalExpnId {
-        self.expn_data().parent.as_local().unwrap()
-    }
 }
 
 impl ExpnId {
@@ -333,7 +312,7 @@ impl ExpnId {
 }
 
 #[derive(Debug)]
-pub struct HygieneData {
+pub(crate) struct HygieneData {
     /// Each expansion should have an associated expansion data, but sometimes there's a delay
     /// between creation of an expansion ID and obtaining its data (e.g. macros are collected
     /// first and then resolved later), so we use an `Option` here.
@@ -384,16 +363,11 @@ impl HygieneData {
         }
     }
 
-    pub fn with<T, F: FnOnce(&mut HygieneData) -> T>(f: F) -> T {
+    fn with<T, F: FnOnce(&mut HygieneData) -> T>(f: F) -> T {
         with_session_globals(|session_globals| f(&mut session_globals.hygiene_data.borrow_mut()))
     }
 
     #[inline]
-    fn local_expn_hash(&self, expn_id: LocalExpnId) -> ExpnHash {
-        self.local_expn_hashes[expn_id]
-    }
-
-    #[inline]
     fn expn_hash(&self, expn_id: ExpnId) -> ExpnHash {
         match expn_id.as_local() {
             Some(expn_id) => self.local_expn_hashes[expn_id],
@@ -746,7 +720,7 @@ impl SyntaxContext {
     }
 
     /// Like `SyntaxContext::adjust`, but also normalizes `self` to macros 2.0.
-    pub fn normalize_to_macros_2_0_and_adjust(&mut self, expn_id: ExpnId) -> Option<ExpnId> {
+    pub(crate) fn normalize_to_macros_2_0_and_adjust(&mut self, expn_id: ExpnId) -> Option<ExpnId> {
         HygieneData::with(|data| {
             *self = data.normalize_to_macros_2_0(*self);
             data.adjust(self, expn_id)
@@ -779,7 +753,11 @@ impl SyntaxContext {
     /// ```
     /// This returns `None` if the context cannot be glob-adjusted.
     /// Otherwise, it returns the scope to use when privacy checking (see `adjust` for details).
-    pub fn glob_adjust(&mut self, expn_id: ExpnId, glob_span: Span) -> Option<Option<ExpnId>> {
+    pub(crate) fn glob_adjust(
+        &mut self,
+        expn_id: ExpnId,
+        glob_span: Span,
+    ) -> Option<Option<ExpnId>> {
         HygieneData::with(|data| {
             let mut scope = None;
             let mut glob_ctxt = data.normalize_to_macros_2_0(glob_span.ctxt());
@@ -803,7 +781,7 @@ impl SyntaxContext {
     ///     assert!(self.glob_adjust(expansion, glob_ctxt) == Some(privacy_checking_scope));
     /// }
     /// ```
-    pub fn reverse_glob_adjust(
+    pub(crate) fn reverse_glob_adjust(
         &mut self,
         expn_id: ExpnId,
         glob_span: Span,
@@ -858,11 +836,11 @@ impl SyntaxContext {
     }
 
     #[inline]
-    pub fn outer_mark(self) -> (ExpnId, Transparency) {
+    fn outer_mark(self) -> (ExpnId, Transparency) {
         HygieneData::with(|data| data.outer_mark(self))
     }
 
-    pub fn dollar_crate_name(self) -> Symbol {
+    pub(crate) fn dollar_crate_name(self) -> Symbol {
         HygieneData::with(|data| data.syntax_context_data[self.0 as usize].dollar_crate_name)
     }
 
@@ -961,12 +939,12 @@ pub struct ExpnData {
     /// The normal module (`mod`) in which the expanded macro was defined.
     pub parent_module: Option<DefId>,
     /// Suppresses the `unsafe_code` lint for code produced by this macro.
-    pub allow_internal_unsafe: bool,
+    pub(crate) allow_internal_unsafe: bool,
     /// Enables the macro helper hack (`ident!(...)` -> `$crate::ident!(...)`) for this macro.
     pub local_inner_macros: bool,
     /// Should debuginfo for the macro be collapsed to the outermost expansion site (in other
     /// words, was the macro definition annotated with `#[collapse_debuginfo]`)?
-    pub collapse_debuginfo: bool,
+    pub(crate) collapse_debuginfo: bool,
 }
 
 impl !PartialEq for ExpnData {}
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
index 1a94b7e98fd..fc13bdff36f 100644
--- a/compiler/rustc_span/src/lib.rs
+++ b/compiler/rustc_span/src/lib.rs
@@ -4,10 +4,12 @@
 //!
 //! - the *span*, represented by [`SpanData`] and related types;
 //! - source code as represented by a [`SourceMap`]; and
-//! - interned strings, represented by [`Symbol`]s, with some common symbols available statically in the [`sym`] module.
+//! - interned strings, represented by [`Symbol`]s, with some common symbols available statically
+//!   in the [`sym`] module.
 //!
-//! Unlike most compilers, the span contains not only the position in the source code, but also various other metadata,
-//! such as the edition and macro hygiene. This metadata is stored in [`SyntaxContext`] and [`ExpnData`].
+//! Unlike most compilers, the span contains not only the position in the source code, but also
+//! various other metadata, such as the edition and macro hygiene. This metadata is stored in
+//! [`SyntaxContext`] and [`ExpnData`].
 //!
 //! ## Note
 //!
@@ -117,7 +119,6 @@ impl SessionGlobals {
     }
 }
 
-#[inline]
 pub fn create_session_globals_then<R>(edition: Edition, f: impl FnOnce() -> R) -> R {
     assert!(
         !SESSION_GLOBALS.is_set(),
@@ -128,7 +129,6 @@ pub fn create_session_globals_then<R>(edition: Edition, f: impl FnOnce() -> R) -
     SESSION_GLOBALS.set(&session_globals, f)
 }
 
-#[inline]
 pub fn set_session_globals_then<R>(session_globals: &SessionGlobals, f: impl FnOnce() -> R) -> R {
     assert!(
         !SESSION_GLOBALS.is_set(),
@@ -138,7 +138,6 @@ pub fn set_session_globals_then<R>(session_globals: &SessionGlobals, f: impl FnO
     SESSION_GLOBALS.set(session_globals, f)
 }
 
-#[inline]
 pub fn create_default_session_if_not_set_then<R, F>(f: F) -> R
 where
     F: FnOnce(&SessionGlobals) -> R,
@@ -146,7 +145,6 @@ where
     create_session_if_not_set_then(edition::DEFAULT_EDITION, f)
 }
 
-#[inline]
 pub fn create_session_if_not_set_then<R, F>(edition: Edition, f: F) -> R
 where
     F: FnOnce(&SessionGlobals) -> R,
@@ -159,7 +157,6 @@ where
     }
 }
 
-#[inline]
 pub fn with_session_globals<R, F>(f: F) -> R
 where
     F: FnOnce(&SessionGlobals) -> R,
@@ -167,7 +164,6 @@ where
     SESSION_GLOBALS.with(f)
 }
 
-#[inline]
 pub fn create_default_session_globals_then<R>(f: impl FnOnce() -> R) -> R {
     create_session_globals_then(edition::DEFAULT_EDITION, f)
 }
@@ -179,8 +175,7 @@ scoped_tls::scoped_thread_local!(static SESSION_GLOBALS: SessionGlobals);
 
 // FIXME: We should use this enum or something like it to get rid of the
 // use of magic `/rust/1.x/...` paths across the board.
-#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd)]
-#[derive(Decodable)]
+#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Decodable)]
 pub enum RealFileName {
     LocalPath(PathBuf),
     /// For remapped paths (namely paths into libstd that have been mapped
@@ -217,8 +212,8 @@ impl<S: Encoder> Encodable<S> for RealFileName {
 
             RealFileName::Remapped { ref local_path, ref virtual_name } => encoder
                 .emit_enum_variant(1, |encoder| {
-                    // For privacy and build reproducibility, we must not embed host-dependant path in artifacts
-                    // if they have been remapped by --remap-path-prefix
+                    // For privacy and build reproducibility, we must not embed host-dependant path
+                    // in artifacts if they have been remapped by --remap-path-prefix
                     assert!(local_path.is_none());
                     local_path.encode(encoder);
                     virtual_name.encode(encoder);
@@ -954,7 +949,7 @@ impl Span {
     /// Produces a span with the same location as `self` and context produced by a macro with the
     /// given ID and transparency, assuming that macro was defined directly and not produced by
     /// some other macro (which is the case for built-in and procedural macros).
-    pub fn with_ctxt_from_mark(self, expn_id: ExpnId, transparency: Transparency) -> Span {
+    fn with_ctxt_from_mark(self, expn_id: ExpnId, transparency: Transparency) -> Span {
         self.with_ctxt(SyntaxContext::root().apply_mark(expn_id, transparency))
     }
 
@@ -1529,7 +1524,8 @@ impl SourceFile {
         })
     }
 
-    /// This converts the `lines` field to contain `SourceFileLines::Lines` if needed and freezes it.
+    /// This converts the `lines` field to contain `SourceFileLines::Lines` if needed and freezes
+    /// it.
     fn convert_diffs_to_lines_frozen(&self) {
         let mut guard = if let Some(guard) = self.lines.try_write() { guard } else { return };
 
@@ -2247,6 +2243,9 @@ where
 
 /// Useful type to use with `Result<>` indicate that an error has already
 /// been reported to the user, so no need to continue checking.
+///
+/// The `()` field is necessary: it is non-`pub`, which means values of this
+/// type cannot be constructed outside of this crate.
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
 #[derive(HashStable_Generic)]
 pub struct ErrorGuaranteed(());
@@ -2264,7 +2263,8 @@ impl<E: rustc_serialize::Encoder> Encodable<E> for ErrorGuaranteed {
     #[inline]
     fn encode(&self, _e: &mut E) {
         panic!(
-            "should never serialize an `ErrorGuaranteed`, as we do not write metadata or incremental caches in case errors occurred"
+            "should never serialize an `ErrorGuaranteed`, as we do not write metadata or \
+            incremental caches in case errors occurred"
         )
     }
 }
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
index 612d396b099..dcf346acb33 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/compiler/rustc_span/src/source_map.rs
@@ -9,21 +9,16 @@
 //! within the `SourceMap`, which upon request can be converted to line and column
 //! information, source code snippets, etc.
 
-pub use crate::hygiene::{ExpnData, ExpnKind};
-pub use crate::*;
-
+use crate::*;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{Hash128, Hash64, StableHasher};
 use rustc_data_structures::sync::{IntoDynSyncSend, Lrc, MappedReadGuard, ReadGuard, RwLock};
 use std::cmp;
+use std::fs;
 use std::hash::Hash;
+use std::io::{self, BorrowedBuf, Read};
 use std::path::{self, Path, PathBuf};
 
-use std::fs;
-use std::io;
-use std::io::BorrowedBuf;
-use std::io::Read;
-
 #[cfg(test)]
 mod tests;
 
@@ -41,7 +36,7 @@ pub fn original_sp(sp: Span, enclosing_sp: Span) -> Span {
     }
 }
 
-pub mod monotonic {
+mod monotonic {
     use std::ops::{Deref, DerefMut};
 
     /// A `MonotonicVec` is a `Vec` which can only be grown.
@@ -51,18 +46,14 @@ pub mod monotonic {
     // field is inaccessible
     pub struct MonotonicVec<T>(Vec<T>);
     impl<T> MonotonicVec<T> {
-        pub fn new(val: Vec<T>) -> MonotonicVec<T> {
-            MonotonicVec(val)
-        }
-
-        pub fn push(&mut self, val: T) {
+        pub(super) fn push(&mut self, val: T) {
             self.0.push(val);
         }
     }
 
     impl<T> Default for MonotonicVec<T> {
         fn default() -> Self {
-            MonotonicVec::new(vec![])
+            MonotonicVec(vec![])
         }
     }
 
@@ -207,7 +198,7 @@ impl StableSourceFileId {
 //
 
 #[derive(Default)]
-pub(super) struct SourceMapFiles {
+struct SourceMapFiles {
     source_files: monotonic::MonotonicVec<Lrc<SourceFile>>,
     stable_id_to_source_file: FxHashMap<StableSourceFileId, Lrc<SourceFile>>,
 }
@@ -466,33 +457,6 @@ impl SourceMap {
         self.span_to_string(sp, FileNameDisplayPreference::Remapped)
     }
 
-    /// Format the span location suitable for pretty printing annotations with relative line numbers
-    pub fn span_to_relative_line_string(&self, sp: Span, relative_to: Span) -> String {
-        if self.files.borrow().source_files.is_empty() || sp.is_dummy() || relative_to.is_dummy() {
-            return "no-location".to_string();
-        }
-
-        let lo = self.lookup_char_pos(sp.lo());
-        let hi = self.lookup_char_pos(sp.hi());
-        let offset = self.lookup_char_pos(relative_to.lo());
-
-        if lo.file.name != offset.file.name || !relative_to.contains(sp) {
-            return self.span_to_embeddable_string(sp);
-        }
-
-        let lo_line = lo.line.saturating_sub(offset.line);
-        let hi_line = hi.line.saturating_sub(offset.line);
-
-        format!(
-            "{}:+{}:{}: +{}:{}",
-            lo.file.name.display(FileNameDisplayPreference::Remapped),
-            lo_line,
-            lo.col.to_usize() + 1,
-            hi_line,
-            hi.col.to_usize() + 1,
-        )
-    }
-
     /// Format the span location to be printed in diagnostics. Must not be emitted
     /// to build artifacts as this may leak local file paths. Use span_to_embeddable_string
     /// for string suitable for embedding.
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index 6a131c0d78b..ff470ce1fa0 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/compiler/rustc_span/src/symbol.rs
@@ -20,8 +20,8 @@ mod tests;
 
 // The proc macro code for this is in `compiler/rustc_macros/src/symbols.rs`.
 symbols! {
-    // After modifying this list adjust `is_special`, `is_used_keyword`/`is_unused_keyword`,
-    // this should be rarely necessary though if the keywords are kept in alphabetic order.
+    // If you modify this list, adjust `is_special` and `is_used_keyword`/`is_unused_keyword`.
+    // But this should rarely be necessary if the keywords are kept in alphabetic order.
     Keywords {
         // Special reserved identifiers used internally for elided lifetimes,
         // unnamed method parameters, crate root module, error recovery etc.
@@ -894,7 +894,7 @@ symbols! {
         inline_const_pat,
         inout,
         instruction_set,
-        integer_: "integer",
+        integer_: "integer", // underscore to avoid clashing with the function `sym::integer` below
         integral,
         into_future,
         into_iter,