Skip to content

Commit

Permalink
Auto merge of rust-lang#84373 - cjgillot:resolve-span, r=michaelwoeri…
Browse files Browse the repository at this point in the history
…ster,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.
  • Loading branch information
bors committed Sep 11, 2021
2 parents 8c2b6ea + 7842b80 commit 547d937
Show file tree
Hide file tree
Showing 68 changed files with 2,822 additions and 1,192 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) }`,
Expand Down
20 changes: 16 additions & 4 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
24 changes: 23 additions & 1 deletion compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
}

Expand All @@ -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);

Expand All @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_interface/src/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(&_)));
}
3 changes: 2 additions & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ pub fn provide(providers: &mut Providers) {
index.parenting.get(&id).copied().unwrap_or(CRATE_HIR_ID)
};
providers.hir_attrs = |tcx, id| AttributeMap { map: &tcx.untracked_crate.attrs, prefix: id };
providers.source_span = |tcx, def_id| tcx.resolutions(()).definitions.def_span(def_id);
providers.def_span = |tcx, def_id| tcx.hir().span_if_local(def_id).unwrap_or(DUMMY_SP);
providers.fn_arg_names = |tcx, id| {
let hir = tcx.hir();
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_hir::definitions::{DefPathHash, Definitions};
use rustc_session::Session;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::Symbol;
use rustc_span::{BytePos, CachingSourceMapView, SourceFile, SpanData};
use rustc_span::{BytePos, CachingSourceMapView, SourceFile, Span, SpanData};

use smallvec::SmallVec;
use std::cmp::Ord;
Expand Down Expand Up @@ -227,6 +227,11 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
self.def_path_hash(def_id)
}

#[inline]
fn def_span(&self, def_id: LocalDefId) -> Span {
self.definitions.def_span(def_id)
}

fn span_data_to_lines_and_cols(
&mut self,
span: &SpanData,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ rustc_queries! {
desc { "get the resolver outputs" }
}

/// Return the span for a definition.
/// Contrary to `def_span` below, this query returns the full absolute span of the definition.
/// This span is meant for dep-tracking rather than diagnostics. It should not be used outside
/// of rustc_middle::hir::source_map.
query source_span(key: LocalDefId) -> Span {
desc { "get the source span" }
}

/// Represents crate as a whole (as distinct from the top-level crate module).
/// If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`),
/// we will have to assume that any change means that you need to be recompiled.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ fn foo(&self) -> Self::T { String::new() }
{
let (span, sugg) = if has_params {
let pos = span.hi() - BytePos(1);
let span = Span::new(pos, pos, span.ctxt());
let span = Span::new(pos, pos, span.ctxt(), span.parent());
(span, format!(", {} = {}", assoc.ident, ty))
} else {
let item_args = self.format_generic_args(assoc_substs);
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,11 @@ impl CoverageSpan {
/// body_span), returns the macro name symbol.
pub fn visible_macro(&self, body_span: Span) -> Option<Symbol> {
if let Some(current_macro) = self.current_macro() {
if self.expn_span.parent().unwrap_or_else(|| bug!("macro must have a parent")).ctxt()
if self
.expn_span
.parent_callsite()
.unwrap_or_else(|| bug!("macro must have a parent"))
.ctxt()
== body_span.ctxt()
{
return Some(current_macro);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fn maybe_source_file_to_parser(
let mut parser = stream_to_parser(sess, stream, None);
parser.unclosed_delims = unclosed_delims;
if parser.token == token::Eof {
parser.token.span = Span::new(end_pos, end_pos, parser.token.span.ctxt());
parser.token.span = Span::new(end_pos, end_pos, parser.token.span.ctxt(), None);
}

Ok(parser)
Expand Down
Loading

0 comments on commit 547d937

Please sign in to comment.