Auto merge of rust-lang#84373 - cjgillot:resolve-span, r=michaelwoerister,petrochenkov

Encode spans relative to the enclosing item

The aim of this PR is to avoid recomputing queries when code is moved without modification.

MCP at rust-lang/compiler-team#443

This is achieved by :
1. storing the HIR owner LocalDefId information inside the span;
2. encoding and decoding spans relative to the enclosing item in the incremental on-disk cache;
3. marking a dependency to the `source_span(LocalDefId)` query when we translate a span from the short (`Span`) representation to its explicit (`SpanData`) representation.

Since all client code uses `Span`, step 3 ensures that all manipulations
of span byte positions actually create the dependency edge between
the caller and the `source_span(LocalDefId)`.
This query return the actual absolute span of the parent item.
As a consequence, any source code motion that changes the absolute byte position of a node will either:
- modify the distance to the parent's beginning, so change the relative span's hash;
- dirty `source_span`, and trigger the incremental recomputation of all code that
  depends on the span's absolute byte position.

With this scheme, I believe the dependency tracking to be accurate.

For the moment, the spans are marked during lowering.
I'd rather do this during def-collection,
but the AST MutVisitor is not practical enough just yet.
The only difference is that we attach macro-expanded spans
to their expansion point instead of the macro itself.

Author: bors

compiler/rustc_ast/src/attr/mod.rs

@@ -367,7 +367,7 @@ impl MetaItem {
             let is_first = i == 0;
             if !is_first {
                 let mod_sep_span =
-                    Span::new(last_pos, segment.ident.span.lo(), segment.ident.span.ctxt());
+                    Span::new(last_pos, segment.ident.span.lo(), segment.ident.span.ctxt(), None);
                 idents.push(TokenTree::token(token::ModSep, mod_sep_span).into());
             }
             idents.push(TokenTree::Token(Token::from_ast_ident(segment.ident)).into());

compiler/rustc_ast_lowering/src/expr.rs

@@ -422,7 +422,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let if_kind = hir::ExprKind::If(new_cond, self.arena.alloc(then), Some(else_expr));
         let if_expr = self.expr(span, if_kind, ThinVec::new());
         let block = self.block_expr(self.arena.alloc(if_expr));
-        hir::ExprKind::Loop(block, opt_label, hir::LoopSource::While, span.with_hi(cond.span.hi()))
+        let span = self.lower_span(span.with_hi(cond.span.hi()));
+        let opt_label = self.lower_label(opt_label);
+        hir::ExprKind::Loop(block, opt_label, hir::LoopSource::While, span)
     }
 
     /// Desugar `try { <stmts>; <expr> }` into `{ <stmts>; ::std::ops::Try::from_output(<expr>) }`,

compiler/rustc_ast_lowering/src/lib.rs

@@ -165,6 +165,8 @@ struct LoweringContext<'a, 'hir: 'a> {
 pub trait ResolverAstLowering {
     fn def_key(&mut self, id: DefId) -> DefKey;
 
+    fn def_span(&self, id: LocalDefId) -> Span;
+
     fn item_generics_num_lifetimes(&self, def: DefId) -> usize;
 
     fn legacy_const_generic_args(&mut self, expr: &Expr) -> Option<Vec<usize>>;
@@ -215,6 +217,11 @@ impl<'a> rustc_span::HashStableContext for LoweringHasher<'a> {
         true
     }
 
+    #[inline]
+    fn def_span(&self, id: LocalDefId) -> Span {
+        self.resolver.def_span(id)
+    }
+
     #[inline]
     fn def_path_hash(&self, def_id: DefId) -> DefPathHash {
         self.resolver.def_path_hash(def_id)
@@ -711,9 +718,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 
     /// Intercept all spans entering HIR.
-    /// For now we are not doing anything with the intercepted spans.
+    /// Mark a span as relative to the current owning item.
     fn lower_span(&self, span: Span) -> Span {
-        span
+        if self.sess.opts.debugging_opts.incremental_relative_spans {
+            span.with_parent(Some(self.current_hir_id_owner.0))
+        } else {
+            // Do not make spans relative when not using incremental compilation.
+            span
+        }
     }
 
     fn lower_ident(&self, ident: Ident) -> Ident {
@@ -781,7 +793,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             node_id,
             DefPathData::LifetimeNs(str_name),
             ExpnId::root(),
-            span,
+            span.with_parent(None),
         );
 
         hir::GenericParam {
@@ -1513,7 +1525,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         def_node_id,
                         DefPathData::LifetimeNs(name.ident().name),
                         ExpnId::root(),
-                        span,
+                        span.with_parent(None),
                     );
 
                     let (name, kind) = match name {

compiler/rustc_expand/src/base.rs

@@ -15,7 +15,7 @@ use rustc_lint_defs::builtin::PROC_MACRO_BACK_COMPAT;
 use rustc_lint_defs::BuiltinLintDiagnostics;
 use rustc_parse::{self, nt_to_tokenstream, parser, MACRO_ARGUMENTS};
 use rustc_session::{parse::ParseSess, Limit, Session};
-use rustc_span::def_id::{CrateNum, DefId};
+use rustc_span::def_id::{CrateNum, DefId, LocalDefId};
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
 use rustc_span::source_map::SourceMap;
@@ -843,6 +843,7 @@ pub type DeriveResolutions = Vec<(ast::Path, Annotatable, Option<Lrc<SyntaxExten
 
 pub trait ResolverExpand {
     fn next_node_id(&mut self) -> NodeId;
+    fn invocation_parent(&self, id: LocalExpnId) -> LocalDefId;
 
     fn resolve_dollar_crates(&mut self);
     fn visit_ast_fragment_with_placeholders(

compiler/rustc_expand/src/expand.rs

@@ -588,7 +588,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         // Resolve `$crate`s in the fragment for pretty-printing.
         self.cx.resolver.resolve_dollar_crates();
 
-        let invocations = {
+        let mut invocations = {
             let mut collector = InvocationCollector {
                 // Non-derive macro invocations cannot see the results of cfg expansion - they
                 // will either be removed along with the item, or invoked before the cfg/cfg_attr
@@ -613,6 +613,19 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             self.cx
                 .resolver
                 .visit_ast_fragment_with_placeholders(self.cx.current_expansion.id, &fragment);
+
+            if self.cx.sess.opts.debugging_opts.incremental_relative_spans {
+                for (invoc, _) in invocations.iter_mut() {
+                    let expn_id = invoc.expansion_data.id;
+                    let parent_def = self.cx.resolver.invocation_parent(expn_id);
+                    let span = match &mut invoc.kind {
+                        InvocationKind::Bang { ref mut span, .. } => span,
+                        InvocationKind::Attr { attr, .. } => &mut attr.span,
+                        InvocationKind::Derive { path, .. } => &mut path.span,
+                    };
+                    *span = span.with_parent(Some(parent_def));
+                }
+            }
         }
 
         (fragment, invocations)

compiler/rustc_expand/src/proc_macro_server.rs

@@ -745,7 +745,7 @@ impl server::Span for Rustc<'_> {
         self.sess.source_map().lookup_char_pos(span.lo()).file
     }
     fn parent(&mut self, span: Self::Span) -> Option<Self::Span> {
-        span.parent()
+        span.parent_callsite()
     }
     fn source(&mut self, span: Self::Span) -> Self::Span {
         span.source_callsite()

compiler/rustc_hir/src/definitions.rs

@@ -14,6 +14,7 @@ use rustc_data_structures::unhash::UnhashMap;
 use rustc_index::vec::IndexVec;
 use rustc_span::hygiene::ExpnId;
 use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_span::Span;
 
 use std::fmt::{self, Write};
 use std::hash::Hash;
@@ -107,6 +108,8 @@ pub struct Definitions {
 
     /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`.
     expansions_that_defined: FxHashMap<LocalDefId, ExpnId>,
+
+    def_id_to_span: IndexVec<LocalDefId, Span>,
 }
 
 /// A unique identifier that we can use to lookup a definition
@@ -324,7 +327,7 @@ impl Definitions {
     }
 
     /// Adds a root definition (no parent) and a few other reserved definitions.
-    pub fn new(stable_crate_id: StableCrateId) -> Definitions {
+    pub fn new(stable_crate_id: StableCrateId, crate_span: Span) -> Definitions {
         let key = DefKey {
             parent: None,
             disambiguated_data: DisambiguatedDefPathData {
@@ -341,11 +344,18 @@ impl Definitions {
         let root = LocalDefId { local_def_index: table.allocate(key, def_path_hash) };
         assert_eq!(root.local_def_index, CRATE_DEF_INDEX);
 
+        let mut def_id_to_span = IndexVec::new();
+        // A relative span's parent must be an absolute span.
+        debug_assert_eq!(crate_span.data_untracked().parent, None);
+        let _root = def_id_to_span.push(crate_span);
+        debug_assert_eq!(_root, root);
+
         Definitions {
             table,
             def_id_to_hir_id: Default::default(),
             hir_id_to_def_id: Default::default(),
             expansions_that_defined: Default::default(),
+            def_id_to_span,
         }
     }
 
@@ -361,6 +371,7 @@ impl Definitions {
         data: DefPathData,
         expn_id: ExpnId,
         mut next_disambiguator: impl FnMut(LocalDefId, DefPathData) -> u32,
+        span: Span,
     ) -> LocalDefId {
         debug!("create_def(parent={:?}, data={:?}, expn_id={:?})", parent, data, expn_id);
 
@@ -385,6 +396,11 @@ impl Definitions {
             self.expansions_that_defined.insert(def_id, expn_id);
         }
 
+        // A relative span's parent must be an absolute span.
+        debug_assert_eq!(span.data_untracked().parent, None);
+        let _id = self.def_id_to_span.push(span);
+        debug_assert_eq!(_id, def_id);
+
         def_id
     }
 
@@ -412,6 +428,12 @@ impl Definitions {
         self.expansions_that_defined.get(&id).copied().unwrap_or_else(ExpnId::root)
     }
 
+    /// Retrieves the span of the given `DefId` if `DefId` is in the local crate.
+    #[inline]
+    pub fn def_span(&self, def_id: LocalDefId) -> Span {
+        self.def_id_to_span[def_id]
+    }
+
     pub fn iter_local_def_id(&self) -> impl Iterator<Item = LocalDefId> + '_ {
         self.def_id_to_hir_id.iter_enumerated().map(|(k, _)| k)
     }

compiler/rustc_interface/src/callbacks.rs

@@ -25,6 +25,16 @@ fn span_debug(span: rustc_span::Span, f: &mut fmt::Formatter<'_>) -> fmt::Result
     })
 }
 
+fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
+    tls::with_opt(|tcx| {
+        if let Some(tcx) = tcx {
+            let _span = tcx.source_span(def_id);
+            // Sanity check: relative span's parent must be an absolute span.
+            debug_assert_eq!(_span.data_untracked().parent, None);
+        }
+    })
+}
+
 /// This is a callback from `rustc_ast` as it cannot access the implicit state
 /// in `rustc_middle` otherwise. It is used to when diagnostic messages are
 /// emitted and stores them in the current query, if there is one.
@@ -56,6 +66,7 @@ fn def_id_debug(def_id: rustc_hir::def_id::DefId, f: &mut fmt::Formatter<'_>) ->
 /// TyCtxt in.
 pub fn setup_callbacks() {
     rustc_span::SPAN_DEBUG.swap(&(span_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
+    rustc_span::SPAN_TRACK.swap(&(track_span_parent as fn(_)));
     rustc_hir::def_id::DEF_ID_DEBUG.swap(&(def_id_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
     TRACK_DIAGNOSTICS.swap(&(track_diagnostic as fn(&_)));
 }

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -536,7 +536,8 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for Span {
         let hi =
             (hi + source_file.translated_source_file.start_pos) - source_file.original_start_pos;
 
-        Ok(Span::new(lo, hi, ctxt))
+        // Do not try to decode parent for foreign spans.
+        Ok(Span::new(lo, hi, ctxt, None))
     }
 }
 

compiler/rustc_middle/src/hir/map/mod.rs

@@ -969,22 +969,12 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
         .iter_enumerated()
         .filter_map(|(def_id, hod)| {
             let def_path_hash = tcx.untracked_resolutions.definitions.def_path_hash(def_id);
-            let mut hasher = StableHasher::new();
-            hod.as_ref()?.hash_stable(&mut hcx, &mut hasher);
-            AttributeMap { map: &tcx.untracked_crate.attrs, prefix: def_id }
-                .hash_stable(&mut hcx, &mut hasher);
-            Some((def_path_hash, hasher.finish()))
+            let hash = hod.as_ref()?.hash;
+            Some((def_path_hash, hash, def_id))
         })
         .collect();
     hir_body_nodes.sort_unstable_by_key(|bn| bn.0);
 
-    let node_hashes = hir_body_nodes.iter().fold(
-        Fingerprint::ZERO,
-        |combined_fingerprint, &(def_path_hash, fingerprint)| {
-            combined_fingerprint.combine(def_path_hash.0.combine(fingerprint))
-        },
-    );
-
     let upstream_crates = upstream_crates(tcx);
 
     // We hash the final, remapped names of all local source files so we
@@ -1004,7 +994,17 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
     source_file_names.sort_unstable();
 
     let mut stable_hasher = StableHasher::new();
-    node_hashes.hash_stable(&mut hcx, &mut stable_hasher);
+    for (def_path_hash, fingerprint, def_id) in hir_body_nodes.iter() {
+        def_path_hash.0.hash_stable(&mut hcx, &mut stable_hasher);
+        fingerprint.hash_stable(&mut hcx, &mut stable_hasher);
+        AttributeMap { map: &tcx.untracked_crate.attrs, prefix: *def_id }
+            .hash_stable(&mut hcx, &mut stable_hasher);
+        if tcx.sess.opts.debugging_opts.incremental_relative_spans {
+            let span = tcx.untracked_resolutions.definitions.def_span(*def_id);
+            debug_assert_eq!(span.parent(), None);
+            span.hash_stable(&mut hcx, &mut stable_hasher);
+        }
+    }
     upstream_crates.hash_stable(&mut hcx, &mut stable_hasher);
     source_file_names.hash_stable(&mut hcx, &mut stable_hasher);
     tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher);