Skip to content

Commit 01a9f26

Browse files
committed
Streamline rustc_span::HashStableContext.
Currently this trait has five methods. But it only really needs three. For example, currently stable hashing of spans is implemented in `rustc_span`, except a couple of sub-operations are delegated to `rustc_query_system`: `def_span` and `span_data_to_lines_and_cols`. These two delegated sub-operations can be reduced to a single delegated operation that does the full hash computation. Likewise, `assert_default_hashing_controls` depends on two delegated sub-operations, `hashing_controls` and `unstable_opts_incremental_ignore_spans`, and can be simplified. I find the resulting code simpler and clearer -- when necessary, we do a whole operation in `rustc_query_system` instead of doing it partly in `rustc_span` and partly in `rustc_query_system`.
1 parent c69e1a0 commit 01a9f26

3 files changed

Lines changed: 122 additions & 138 deletions

File tree

compiler/rustc_query_system/src/ich/hcx.rs

Lines changed: 108 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
use rustc_data_structures::stable_hasher::HashingControls;
1+
use std::hash::Hash;
2+
3+
use rustc_data_structures::stable_hasher::{HashStable, HashingControls, StableHasher};
24
use rustc_hir::def_id::{DefId, LocalDefId};
35
use rustc_hir::definitions::DefPathHash;
46
use rustc_session::Session;
57
use rustc_session::cstore::Untracked;
68
use rustc_span::source_map::SourceMap;
7-
use rustc_span::{BytePos, CachingSourceMapView, DUMMY_SP, SourceFile, Span, SpanData};
9+
use rustc_span::{CachingSourceMapView, DUMMY_SP, Pos, Span};
810

911
// Very often, we are hashing something that does not need the `CachingSourceMapView`, so we
1012
// initialize it lazily.
@@ -60,16 +62,99 @@ impl<'a> StableHashingContext<'a> {
6062
}
6163
}
6264

65+
#[inline]
66+
fn def_span(&self, def_id: LocalDefId) -> Span {
67+
self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
68+
}
69+
6370
#[inline]
6471
pub fn hashing_controls(&self) -> HashingControls {
6572
self.hashing_controls
6673
}
6774
}
6875

6976
impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
70-
#[inline]
71-
fn unstable_opts_incremental_ignore_spans(&self) -> bool {
72-
self.incremental_ignore_spans
77+
/// Hashes a span in a stable way. We can't directly hash the span's `BytePos` fields (that
78+
/// would be similar to hashing pointers, since those are just offsets into the `SourceMap`).
79+
/// Instead, we hash the (file name, line, column) triple, which stays the same even if the
80+
/// containing `SourceFile` has moved within the `SourceMap`.
81+
///
82+
/// Also note that we are hashing byte offsets for the column, not unicode codepoint offsets.
83+
/// For the purpose of the hash that's sufficient. Also, hashing filenames is expensive so we
84+
/// avoid doing it twice when the span starts and ends in the same file, which is almost always
85+
/// the case.
86+
///
87+
/// IMPORTANT: changes to this method should be reflected in implementations of `SpanEncoder`.
88+
fn span_hash_stable(&mut self, span: Span, hasher: &mut StableHasher) {
89+
const TAG_VALID_SPAN: u8 = 0;
90+
const TAG_INVALID_SPAN: u8 = 1;
91+
const TAG_RELATIVE_SPAN: u8 = 2;
92+
93+
if !self.hashing_controls().hash_spans {
94+
return;
95+
}
96+
97+
let span = span.data_untracked();
98+
span.ctxt.hash_stable(self, hasher);
99+
span.parent.hash_stable(self, hasher);
100+
101+
if span.is_dummy() {
102+
Hash::hash(&TAG_INVALID_SPAN, hasher);
103+
return;
104+
}
105+
106+
let parent = span.parent.map(|parent| self.def_span(parent).data_untracked());
107+
if let Some(parent) = parent
108+
&& parent.contains(span)
109+
{
110+
// This span is enclosed in a definition: only hash the relative position. This catches
111+
// a subset of the cases from the `file.contains(parent.lo)`. But we can do this check
112+
// cheaply without the expensive `span_data_to_lines_and_cols` query.
113+
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
114+
(span.lo - parent.lo).to_u32().hash_stable(self, hasher);
115+
(span.hi - parent.lo).to_u32().hash_stable(self, hasher);
116+
return;
117+
}
118+
119+
// If this is not an empty or invalid span, we want to hash the last position that belongs
120+
// to it, as opposed to hashing the first position past it.
121+
let Some((file, line_lo, col_lo, line_hi, col_hi)) =
122+
self.source_map().span_data_to_lines_and_cols(&span)
123+
else {
124+
Hash::hash(&TAG_INVALID_SPAN, hasher);
125+
return;
126+
};
127+
128+
if let Some(parent) = parent
129+
&& file.contains(parent.lo)
130+
{
131+
// This span is relative to another span in the same file,
132+
// only hash the relative position.
133+
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
134+
Hash::hash(&(span.lo.0.wrapping_sub(parent.lo.0)), hasher);
135+
Hash::hash(&(span.hi.0.wrapping_sub(parent.lo.0)), hasher);
136+
return;
137+
}
138+
139+
Hash::hash(&TAG_VALID_SPAN, hasher);
140+
Hash::hash(&file.stable_id, hasher);
141+
142+
// Hash both the length and the end location (line/column) of a span. If we hash only the
143+
// length, for example, then two otherwise equal spans with different end locations will
144+
// have the same hash. This can cause a problem during incremental compilation wherein a
145+
// previous result for a query that depends on the end location of a span will be
146+
// incorrectly reused when the end location of the span it depends on has changed (see
147+
// issue #74890). A similar analysis applies if some query depends specifically on the
148+
// length of the span, but we only hash the end location. So hash both.
149+
150+
let col_lo_trunc = (col_lo.0 as u64) & 0xFF;
151+
let line_lo_trunc = ((line_lo as u64) & 0xFF_FF_FF) << 8;
152+
let col_hi_trunc = (col_hi.0 as u64) & 0xFF << 32;
153+
let line_hi_trunc = ((line_hi as u64) & 0xFF_FF_FF) << 40;
154+
let col_line = col_lo_trunc | line_lo_trunc | col_hi_trunc | line_hi_trunc;
155+
let len = (span.hi - span.lo).0;
156+
Hash::hash(&col_line, hasher);
157+
Hash::hash(&len, hasher);
73158
}
74159

75160
#[inline]
@@ -81,22 +166,25 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
81166
}
82167
}
83168

84-
#[inline]
85-
fn def_span(&self, def_id: LocalDefId) -> Span {
86-
self.untracked.source_span.get(def_id).unwrap_or(DUMMY_SP)
87-
}
88-
89-
#[inline]
90-
fn span_data_to_lines_and_cols(
91-
&mut self,
92-
span: &SpanData,
93-
) -> Option<(&SourceFile, usize, BytePos, usize, BytePos)> {
94-
self.source_map().span_data_to_lines_and_cols(span)
95-
}
169+
/// Assert that the provided `HashStableContext` is configured with the default
170+
/// `HashingControls`. We should always have bailed out before getting to here with a
171+
/// non-default mode. With this check in place, we can avoid the need to maintain separate
172+
/// versions of `ExpnData` hashes for each permutation of `HashingControls` settings.
173+
fn assert_default_hashing_controls(&self, msg: &str) {
174+
let hashing_controls = self.hashing_controls;
175+
let HashingControls { hash_spans } = hashing_controls;
96176

97-
#[inline]
98-
fn hashing_controls(&self) -> HashingControls {
99-
self.hashing_controls
177+
// Note that we require that `hash_spans` be the inverse of the global `-Z
178+
// incremental-ignore-spans` option. Normally, this option is disabled, in which case
179+
// `hash_spans` must be true.
180+
//
181+
// Span hashing can also be disabled without `-Z incremental-ignore-spans`. This is the
182+
// case for instance when building a hash for name mangling. Such configuration must not be
183+
// used for metadata.
184+
assert_eq!(
185+
hash_spans, !self.incremental_ignore_spans,
186+
"Attempted hashing of {msg} with non-default HashingControls: {hashing_controls:?}"
187+
);
100188
}
101189
}
102190

compiler/rustc_span/src/hygiene.rs

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use std::{fmt, iter, mem};
3030

3131
use rustc_data_structures::fingerprint::Fingerprint;
3232
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
33-
use rustc_data_structures::stable_hasher::{HashStable, HashingControls, StableHasher};
33+
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
3434
use rustc_data_structures::sync::Lock;
3535
use rustc_data_structures::unhash::UnhashMap;
3636
use rustc_hashes::Hash64;
@@ -126,29 +126,6 @@ rustc_index::newtype_index! {
126126
impl !Ord for LocalExpnId {}
127127
impl !PartialOrd for LocalExpnId {}
128128

129-
/// Assert that the provided `HashStableContext` is configured with the 'default'
130-
/// `HashingControls`. We should always have bailed out before getting to here
131-
/// with a non-default mode. With this check in place, we can avoid the need
132-
/// to maintain separate versions of `ExpnData` hashes for each permutation
133-
/// of `HashingControls` settings.
134-
fn assert_default_hashing_controls(ctx: &impl HashStableContext, msg: &str) {
135-
let hashing_controls = ctx.hashing_controls();
136-
let HashingControls { hash_spans } = hashing_controls;
137-
138-
// Note that we require that `hash_spans` be the inverse of the global
139-
// `-Z incremental-ignore-spans` option. Normally, this option is disabled,
140-
// in which case `hash_spans` must be true.
141-
//
142-
// Span hashing can also be disabled without `-Z incremental-ignore-spans`.
143-
// This is the case for instance when building a hash for name mangling.
144-
// Such configuration must not be used for metadata.
145-
assert_eq!(
146-
hash_spans,
147-
!ctx.unstable_opts_incremental_ignore_spans(),
148-
"Attempted hashing of {msg} with non-default HashingControls: {hashing_controls:?}"
149-
);
150-
}
151-
152129
/// A unique hash value associated to an expansion.
153130
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, Encodable, Decodable, HashStable_Generic)]
154131
pub struct ExpnHash(Fingerprint);
@@ -1508,7 +1485,7 @@ pub fn raw_encode_syntax_context(
15081485
fn update_disambiguator(expn_data: &mut ExpnData, mut ctx: impl HashStableContext) -> ExpnHash {
15091486
// This disambiguator should not have been set yet.
15101487
assert_eq!(expn_data.disambiguator, 0, "Already set disambiguator for ExpnData: {expn_data:?}");
1511-
assert_default_hashing_controls(&ctx, "ExpnData (disambiguator)");
1488+
ctx.assert_default_hashing_controls("ExpnData (disambiguator)");
15121489
let mut expn_hash = expn_data.hash_expn(&mut ctx);
15131490

15141491
let disambiguator = HygieneData::with(|data| {
@@ -1558,7 +1535,7 @@ impl<CTX: HashStableContext> HashStable<CTX> for SyntaxContext {
15581535

15591536
impl<CTX: HashStableContext> HashStable<CTX> for ExpnId {
15601537
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
1561-
assert_default_hashing_controls(ctx, "ExpnId");
1538+
ctx.assert_default_hashing_controls("ExpnId");
15621539
let hash = if *self == ExpnId::root() {
15631540
// Avoid fetching TLS storage for a trivial often-used value.
15641541
Fingerprint::ZERO

compiler/rustc_span/src/lib.rs

Lines changed: 11 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ use hygiene::Transparency;
5454
pub use hygiene::{
5555
DesugaringKind, ExpnData, ExpnHash, ExpnId, ExpnKind, LocalExpnId, MacroKind, SyntaxContext,
5656
};
57-
use rustc_data_structures::stable_hasher::HashingControls;
5857
pub mod def_id;
5958
use def_id::{CrateNum, DefId, DefIndex, DefPathHash, LOCAL_CRATE, LocalDefId, StableCrateId};
6059
pub mod edit_distance;
@@ -2798,105 +2797,25 @@ impl InnerSpan {
27982797
/// This is a hack to allow using the [`HashStable_Generic`] derive macro
27992798
/// instead of implementing everything in rustc_middle.
28002799
pub trait HashStableContext {
2800+
/// The main event: stable hashing of a span.
2801+
fn span_hash_stable(&mut self, span: Span, hasher: &mut StableHasher);
2802+
2803+
/// Compute a `DefPathHash`.
28012804
fn def_path_hash(&self, def_id: DefId) -> DefPathHash;
2802-
/// Accesses `sess.opts.unstable_opts.incremental_ignore_spans` since
2803-
/// we don't have easy access to a `Session`
2804-
fn unstable_opts_incremental_ignore_spans(&self) -> bool;
2805-
fn def_span(&self, def_id: LocalDefId) -> Span;
2806-
fn span_data_to_lines_and_cols(
2807-
&mut self,
2808-
span: &SpanData,
2809-
) -> Option<(&SourceFile, usize, BytePos, usize, BytePos)>;
2810-
fn hashing_controls(&self) -> HashingControls;
2805+
2806+
/// Assert that the provided `HashStableContext` is configured with the default
2807+
/// `HashingControls`. We should always have bailed out before getting to here with a
2808+
fn assert_default_hashing_controls(&self, msg: &str);
28112809
}
28122810

28132811
impl<CTX> HashStable<CTX> for Span
28142812
where
28152813
CTX: HashStableContext,
28162814
{
2817-
/// Hashes a span in a stable way. We can't directly hash the span's `BytePos`
2818-
/// fields (that would be similar to hashing pointers, since those are just
2819-
/// offsets into the `SourceMap`). Instead, we hash the (file name, line, column)
2820-
/// triple, which stays the same even if the containing `SourceFile` has moved
2821-
/// within the `SourceMap`.
2822-
///
2823-
/// Also note that we are hashing byte offsets for the column, not unicode
2824-
/// codepoint offsets. For the purpose of the hash that's sufficient.
2825-
/// Also, hashing filenames is expensive so we avoid doing it twice when the
2826-
/// span starts and ends in the same file, which is almost always the case.
2827-
///
2828-
/// IMPORTANT: changes to this method should be reflected in implementations of `SpanEncoder`.
2815+
#[inline]
28292816
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
2830-
const TAG_VALID_SPAN: u8 = 0;
2831-
const TAG_INVALID_SPAN: u8 = 1;
2832-
const TAG_RELATIVE_SPAN: u8 = 2;
2833-
2834-
if !ctx.hashing_controls().hash_spans {
2835-
return;
2836-
}
2837-
2838-
let span = self.data_untracked();
2839-
span.ctxt.hash_stable(ctx, hasher);
2840-
span.parent.hash_stable(ctx, hasher);
2841-
2842-
if span.is_dummy() {
2843-
Hash::hash(&TAG_INVALID_SPAN, hasher);
2844-
return;
2845-
}
2846-
2847-
let parent = span.parent.map(|parent| ctx.def_span(parent).data_untracked());
2848-
if let Some(parent) = parent
2849-
&& parent.contains(span)
2850-
{
2851-
// This span is enclosed in a definition: only hash the relative position.
2852-
// This catches a subset of the cases from the `file.contains(parent.lo)`,
2853-
// But we can do this check cheaply without the expensive `span_data_to_lines_and_cols` query.
2854-
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
2855-
(span.lo - parent.lo).to_u32().hash_stable(ctx, hasher);
2856-
(span.hi - parent.lo).to_u32().hash_stable(ctx, hasher);
2857-
return;
2858-
}
2859-
2860-
// If this is not an empty or invalid span, we want to hash the last
2861-
// position that belongs to it, as opposed to hashing the first
2862-
// position past it.
2863-
let Some((file, line_lo, col_lo, line_hi, col_hi)) = ctx.span_data_to_lines_and_cols(&span)
2864-
else {
2865-
Hash::hash(&TAG_INVALID_SPAN, hasher);
2866-
return;
2867-
};
2868-
2869-
if let Some(parent) = parent
2870-
&& file.contains(parent.lo)
2871-
{
2872-
// This span is relative to another span in the same file,
2873-
// only hash the relative position.
2874-
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
2875-
Hash::hash(&(span.lo.0.wrapping_sub(parent.lo.0)), hasher);
2876-
Hash::hash(&(span.hi.0.wrapping_sub(parent.lo.0)), hasher);
2877-
return;
2878-
}
2879-
2880-
Hash::hash(&TAG_VALID_SPAN, hasher);
2881-
Hash::hash(&file.stable_id, hasher);
2882-
2883-
// Hash both the length and the end location (line/column) of a span. If we
2884-
// hash only the length, for example, then two otherwise equal spans with
2885-
// different end locations will have the same hash. This can cause a problem
2886-
// during incremental compilation wherein a previous result for a query that
2887-
// depends on the end location of a span will be incorrectly reused when the
2888-
// end location of the span it depends on has changed (see issue #74890). A
2889-
// similar analysis applies if some query depends specifically on the length
2890-
// of the span, but we only hash the end location. So hash both.
2891-
2892-
let col_lo_trunc = (col_lo.0 as u64) & 0xFF;
2893-
let line_lo_trunc = ((line_lo as u64) & 0xFF_FF_FF) << 8;
2894-
let col_hi_trunc = (col_hi.0 as u64) & 0xFF << 32;
2895-
let line_hi_trunc = ((line_hi as u64) & 0xFF_FF_FF) << 40;
2896-
let col_line = col_lo_trunc | line_lo_trunc | col_hi_trunc | line_hi_trunc;
2897-
let len = (span.hi - span.lo).0;
2898-
Hash::hash(&col_line, hasher);
2899-
Hash::hash(&len, hasher);
2817+
// `span_hash_stable` does all the work.
2818+
ctx.span_hash_stable(*self, hasher)
29002819
}
29012820
}
29022821

0 commit comments

Comments
 (0)