From f471d5f65a960c4363c05449bbf5d6a225a0dded Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 22 May 2025 21:12:26 +0000 Subject: [PATCH 1/2] Cache start and end point --- compiler/rustc_ast_lowering/src/expr.rs | 4 ++-- compiler/rustc_ast_lowering/src/lib.rs | 2 +- compiler/rustc_middle/src/query/erase.rs | 1 + compiler/rustc_middle/src/query/keys.rs | 8 ++++++++ compiler/rustc_middle/src/query/mod.rs | 4 ++++ compiler/rustc_middle/src/ty/context.rs | 12 ++++++++++++ compiler/rustc_mir_build/src/builder/matches/mod.rs | 2 +- compiler/rustc_mir_build/src/builder/scope.rs | 4 ++-- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_span/src/source_map.rs | 3 +-- 10 files changed, 33 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 9f3aed9216c2d..35387ced2c7f4 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -641,7 +641,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } else { let try_span = this.mark_span_with_reason( DesugaringKind::TryBlock, - this.tcx.sess.source_map().end_point(body.span), + this.tcx.end_point(body.span), Some(Arc::clone(&this.allow_try_trait)), ); @@ -1968,7 +1968,7 @@ impl<'hir> LoweringContext<'_, 'hir> { span, Some(Arc::clone(&self.allow_try_trait)), ); - let try_span = self.tcx.sess.source_map().end_point(span); + let try_span = self.tcx.end_point(span); let try_span = self.mark_span_with_reason( DesugaringKind::QuestionMark, try_span, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 422e79ca82ffd..0f308e730d91d 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1402,7 +1402,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } else { self.next_node_id() }; - let span = self.tcx.sess.source_map().start_point(t.span).shrink_to_hi(); + let span = self.tcx.start_point(t.span).shrink_to_hi(); let region = Lifetime { ident: Ident::new(kw::UnderscoreLifetime, span), id }; (region, LifetimeSyntax::Hidden) } diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 5bd111fa2f22d..f74ca10f70436 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -333,6 +333,7 @@ trivial! { rustc_span::ExpnHash, rustc_span::ExpnId, rustc_span::Span, + (rustc_span::Span, rustc_span::Span), rustc_span::Symbol, rustc_span::Ident, rustc_target::spec::PanicStrategy, diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index 9ed1f10455ad3..9ba808fd48380 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -70,6 +70,14 @@ impl Key for () { } } +impl Key for Span { + type Cache = DefaultCache; + + fn default_span(&self, _: TyCtxt<'_>) -> Span { + *self + } +} + impl<'tcx> Key for ty::InstanceKind<'tcx> { type Cache = DefaultCache; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b2133fea08cc3..025a24396c771 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -159,6 +159,10 @@ rustc_queries! { desc { "getting the source span" } } + query start_and_end_point(key: Span) -> (Span, Span) { + desc { "computing the start and end points for span" } + } + /// Represents crate as a whole (as distinct from the top-level crate module). /// /// If you call `tcx.hir_crate(())` we will have to assume that any change diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 0759fa3da428a..321845e73c4e5 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2116,6 +2116,14 @@ impl<'tcx> TyCtxt<'tcx> { self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP) } + pub fn start_point(self, span: Span) -> Span { + self.start_and_end_point(span).0 + } + + pub fn end_point(self, span: Span) -> Span { + self.start_and_end_point(span).1 + } + #[inline(always)] pub fn with_stable_hashing_context( self, @@ -3430,6 +3438,10 @@ pub fn provide(providers: &mut Providers) { tcx.lang_items().panic_impl().is_some_and(|did| did.is_local()) }; providers.source_span = |tcx, def_id| tcx.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP); + providers.start_and_end_point = |tcx, span| { + let sm = tcx.sess.source_map(); + (sm.start_point(span), sm.end_point(span)) + }; } pub fn contains_name(attrs: &[Attribute], name: Symbol) -> bool { diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 977d4f3e931b5..edb9cdb1e4201 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -2489,7 +2489,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); let source_info = self.source_info(guard_span); - let guard_end = self.source_info(tcx.sess.source_map().end_point(guard_span)); + let guard_end = self.source_info(tcx.end_point(guard_span)); let guard_frame = self.guard_context.pop().unwrap(); debug!("Exiting guard building context with locals: {:?}", guard_frame); diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index 2a30777e98c4f..f0333d29a39f1 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/compiler/rustc_mir_build/src/builder/scope.rs @@ -1165,7 +1165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if scope.region_scope == region_scope { let region_scope_span = region_scope.span(self.tcx, self.region_scope_tree); // Attribute scope exit drops to scope's closing brace. - let scope_end = self.tcx.sess.source_map().end_point(region_scope_span); + let scope_end = self.tcx.end_point(region_scope_span); scope.drops.push(DropData { source_info: SourceInfo { span: scope_end, scope: scope.source_scope }, @@ -1196,7 +1196,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scope.invalidate_cache(); if scope.region_scope == region_scope { let region_scope_span = region_scope.span(self.tcx, self.region_scope_tree); - let scope_end = self.tcx.sess.source_map().end_point(region_scope_span); + let scope_end = self.tcx.end_point(region_scope_span); scope.drops.push(DropData { source_info: SourceInfo { span: scope_end, scope: scope.source_scope }, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index fd977a8eb6c0b..7f6a24cdadc43 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -831,7 +831,7 @@ impl<'ra: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'r // Elided lifetime in reference: we resolve as if there was some lifetime `'_` with // NodeId `ty.id`. // This span will be used in case of elision failure. - let span = self.r.tcx.sess.source_map().start_point(ty.span); + let span = self.r.tcx.start_point(ty.span); self.resolve_elided_lifetime(ty.id, span); visit::walk_ty(self, ty); } diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 8a3644163caf3..adf072e5835e6 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -886,8 +886,7 @@ impl SourceMap { /// Returns a new span representing just the first character of the given span. pub fn start_point(&self, sp: Span) -> Span { let width = { - let sp = sp.data(); - let local_begin = self.lookup_byte_offset(sp.lo); + let local_begin = self.lookup_byte_offset(sp.lo()); let start_index = local_begin.pos.to_usize(); let src = local_begin.sf.external_src.read(); From 88486c3dde893484ae34cb8135f2d4841cd874aa Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 22 May 2025 22:19:24 +0000 Subject: [PATCH 2/2] make Span's HashStable impl actually valid i think --- compiler/rustc_span/src/lib.rs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 906462a0d229d..fb9997d515a78 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2641,6 +2641,7 @@ where if def_span.contains(span) { // This span is enclosed in a definition: only hash the relative position. Hash::hash(&TAG_RELATIVE_SPAN, hasher); + parent.hash_stable(ctx, hasher); (span.lo - def_span.lo).to_u32().hash_stable(ctx, hasher); (span.hi - def_span.lo).to_u32().hash_stable(ctx, hasher); return; @@ -2650,31 +2651,16 @@ where // If this is not an empty or invalid span, we want to hash the last // position that belongs to it, as opposed to hashing the first // position past it. - let Some((file, line_lo, col_lo, line_hi, col_hi)) = ctx.span_data_to_lines_and_cols(&span) - else { + let Some((file, line_lo, col_lo, ..)) = ctx.span_data_to_lines_and_cols(&span) else { Hash::hash(&TAG_INVALID_SPAN, hasher); return; }; Hash::hash(&TAG_VALID_SPAN, hasher); Hash::hash(&file.stable_id, hasher); - - // Hash both the length and the end location (line/column) of a span. If we - // hash only the length, for example, then two otherwise equal spans with - // different end locations will have the same hash. This can cause a problem - // during incremental compilation wherein a previous result for a query that - // depends on the end location of a span will be incorrectly reused when the - // end location of the span it depends on has changed (see issue #74890). A - // similar analysis applies if some query depends specifically on the length - // of the span, but we only hash the end location. So hash both. - - let col_lo_trunc = (col_lo.0 as u64) & 0xFF; - let line_lo_trunc = ((line_lo as u64) & 0xFF_FF_FF) << 8; - let col_hi_trunc = (col_hi.0 as u64) & 0xFF << 32; - let line_hi_trunc = ((line_hi as u64) & 0xFF_FF_FF) << 40; - let col_line = col_lo_trunc | line_lo_trunc | col_hi_trunc | line_hi_trunc; + Hash::hash(&line_lo, hasher); + Hash::hash(&col_lo, hasher); let len = (span.hi - span.lo).0; - Hash::hash(&col_line, hasher); Hash::hash(&len, hasher); } }