Auto merge of #84762 - cjgillot:resolve-span-opt, r=petrochenkov

Encode spans relative to the enclosing item -- enable on nightly

Follow-up to #84373 with the flag `-Zincremental-relative-spans` set by default.

This PR seeks to remove one of the main shortcomings of incremental: the handling of spans.
Changing the contents of a function may require redoing part of the compilation process for another function in another file because of span information is changed.
Within one file: all the spans in HIR change, so typechecking had to be re-done.
Between files: spans of associated types/consts/functions change, so type-based resolution needs to be re-done (hygiene information is stored in the span).

The flag `-Zincremental-relative-spans` encodes local spans relative to the span of an item, stored inside the `source_span` query.

Trap: stashed diagnostics are referenced by the "raw" span, so stealing them requires to remove the span's parent.

In order to avoid too much traffic in the span interner, span encoding uses the `ctxt_or_tag` field to encode:
- the parent when the `SyntaxContext` is 0;
- the `SyntaxContext` when the parent is `None`.
Even with this, the PR creates a lot of traffic to the Span interner, when a Span has both a LocalDefId parent and a non-root SyntaxContext. They appear in lowering, when we add a parent to all spans, including those which come from macros, and during inlining when we mark inlined spans.

The last commit changes how queries of `LocalDefId` manage their cache. I can put this in a separate PR if required.

Possible future directions:
- validate that all spans are marked in HIR validation;
- mark macro-expanded spans relative to the def-site and not the use-site.

Author: bors

compiler/rustc_ast_lowering/src/lib.rs

@@ -776,7 +776,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     /// Intercept all spans entering HIR.
     /// Mark a span as relative to the current owning item.
     fn lower_span(&self, span: Span) -> Span {
-        if self.tcx.sess.opts.unstable_opts.incremental_relative_spans {
+        if self.tcx.sess.opts.incremental_relative_spans() {
             span.with_parent(Some(self.current_hir_id_owner.def_id))
         } else {
             // Do not make spans relative when not using incremental compilation.

compiler/rustc_errors/src/lib.rs

@@ -469,10 +469,12 @@ pub enum StashKey {
     CallAssocMethod,
 }
 
-fn default_track_diagnostic(_: &Diagnostic) {}
+fn default_track_diagnostic(d: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnostic)) {
+    (*f)(d)
+}
 
-pub static TRACK_DIAGNOSTICS: AtomicRef<fn(&Diagnostic)> =
-    AtomicRef::new(&(default_track_diagnostic as fn(&_)));
+pub static TRACK_DIAGNOSTICS: AtomicRef<fn(&mut Diagnostic, &mut dyn FnMut(&mut Diagnostic))> =
+    AtomicRef::new(&(default_track_diagnostic as _));
 
 #[derive(Copy, Clone, Default)]
 pub struct HandlerFlags {
@@ -654,17 +656,19 @@ impl Handler {
     /// Retrieve a stashed diagnostic with `steal_diagnostic`.
     pub fn stash_diagnostic(&self, span: Span, key: StashKey, diag: Diagnostic) {
         let mut inner = self.inner.borrow_mut();
-        inner.stash((span, key), diag);
+        inner.stash((span.with_parent(None), key), diag);
     }
 
     /// Steal a previously stashed diagnostic with the given `Span` and [`StashKey`] as the key.
     pub fn steal_diagnostic(&self, span: Span, key: StashKey) -> Option<DiagnosticBuilder<'_, ()>> {
         let mut inner = self.inner.borrow_mut();
-        inner.steal((span, key)).map(|diag| DiagnosticBuilder::new_diagnostic(self, diag))
+        inner
+            .steal((span.with_parent(None), key))
+            .map(|diag| DiagnosticBuilder::new_diagnostic(self, diag))
     }
 
     pub fn has_stashed_diagnostic(&self, span: Span, key: StashKey) -> bool {
-        self.inner.borrow().stashed_diagnostics.get(&(span, key)).is_some()
+        self.inner.borrow().stashed_diagnostics.get(&(span.with_parent(None), key)).is_some()
     }
 
     /// Emit all stashed diagnostics.
@@ -1293,67 +1297,69 @@ impl HandlerInner {
             && !diagnostic.is_force_warn()
         {
             if diagnostic.has_future_breakage() {
-                (*TRACK_DIAGNOSTICS)(diagnostic);
+                (*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
             }
             return None;
         }
 
-        (*TRACK_DIAGNOSTICS)(diagnostic);
-
         if matches!(diagnostic.level, Level::Expect(_) | Level::Allow) {
+            (*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
             return None;
         }
 
-        if let Some(ref code) = diagnostic.code {
-            self.emitted_diagnostic_codes.insert(code.clone());
-        }
-
-        let already_emitted = |this: &mut Self| {
-            let mut hasher = StableHasher::new();
-            diagnostic.hash(&mut hasher);
-            let diagnostic_hash = hasher.finish();
-            !this.emitted_diagnostics.insert(diagnostic_hash)
-        };
+        let mut guaranteed = None;
+        (*TRACK_DIAGNOSTICS)(diagnostic, &mut |diagnostic| {
+            if let Some(ref code) = diagnostic.code {
+                self.emitted_diagnostic_codes.insert(code.clone());
+            }
 
-        // Only emit the diagnostic if we've been asked to deduplicate or
-        // haven't already emitted an equivalent diagnostic.
-        if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
-            debug!(?diagnostic);
-            debug!(?self.emitted_diagnostics);
-            let already_emitted_sub = |sub: &mut SubDiagnostic| {
-                debug!(?sub);
-                if sub.level != Level::OnceNote {
-                    return false;
-                }
+            let already_emitted = |this: &mut Self| {
                 let mut hasher = StableHasher::new();
-                sub.hash(&mut hasher);
+                diagnostic.hash(&mut hasher);
                 let diagnostic_hash = hasher.finish();
-                debug!(?diagnostic_hash);
-                !self.emitted_diagnostics.insert(diagnostic_hash)
+                !this.emitted_diagnostics.insert(diagnostic_hash)
             };
 
-            diagnostic.children.drain_filter(already_emitted_sub).for_each(|_| {});
-
-            self.emitter.emit_diagnostic(diagnostic);
-            if diagnostic.is_error() {
-                self.deduplicated_err_count += 1;
-            } else if let Warning(_) = diagnostic.level {
-                self.deduplicated_warn_count += 1;
+            // Only emit the diagnostic if we've been asked to deduplicate or
+            // haven't already emitted an equivalent diagnostic.
+            if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
+                debug!(?diagnostic);
+                debug!(?self.emitted_diagnostics);
+                let already_emitted_sub = |sub: &mut SubDiagnostic| {
+                    debug!(?sub);
+                    if sub.level != Level::OnceNote {
+                        return false;
+                    }
+                    let mut hasher = StableHasher::new();
+                    sub.hash(&mut hasher);
+                    let diagnostic_hash = hasher.finish();
+                    debug!(?diagnostic_hash);
+                    !self.emitted_diagnostics.insert(diagnostic_hash)
+                };
+
+                diagnostic.children.drain_filter(already_emitted_sub).for_each(|_| {});
+
+                self.emitter.emit_diagnostic(diagnostic);
+                if diagnostic.is_error() {
+                    self.deduplicated_err_count += 1;
+                } else if let Warning(_) = diagnostic.level {
+                    self.deduplicated_warn_count += 1;
+                }
             }
-        }
-        if diagnostic.is_error() {
-            if matches!(diagnostic.level, Level::Error { lint: true }) {
-                self.bump_lint_err_count();
+            if diagnostic.is_error() {
+                if matches!(diagnostic.level, Level::Error { lint: true }) {
+                    self.bump_lint_err_count();
+                } else {
+                    self.bump_err_count();
+                }
+
+                guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
             } else {
-                self.bump_err_count();
+                self.bump_warn_count();
             }
+        });
 
-            Some(ErrorGuaranteed::unchecked_claim_error_was_emitted())
-        } else {
-            self.bump_warn_count();
-
-            None
-        }
+        guaranteed
     }
 
     fn emit_artifact_notification(&mut self, path: &Path, artifact_type: &str) {

compiler/rustc_expand/src/expand.rs

@@ -587,7 +587,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 .resolver
                 .visit_ast_fragment_with_placeholders(self.cx.current_expansion.id, &fragment);
 
-            if self.cx.sess.opts.unstable_opts.incremental_relative_spans {
+            if self.cx.sess.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);

compiler/rustc_hir_typeck/src/expr.rs

@@ -395,7 +395,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             E0614,
                             "type `{oprnd_t}` cannot be dereferenced",
                         );
-                        let sp = tcx.sess.source_map().start_point(expr.span);
+                        let sp = tcx.sess.source_map().start_point(expr.span).with_parent(None);
                         if let Some(sp) =
                             tcx.sess.parse_sess.ambiguous_block_expr_parse.borrow().get(&sp)
                         {

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -974,7 +974,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err: &mut Diagnostic,
         expr: &hir::Expr<'_>,
     ) -> bool {
-        let sp = self.tcx.sess.source_map().start_point(expr.span);
+        let sp = self.tcx.sess.source_map().start_point(expr.span).with_parent(None);
         if let Some(sp) = self.tcx.sess.parse_sess.ambiguous_block_expr_parse.borrow().get(&sp) {
             // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`
             err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -339,7 +339,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         &mut err, item_name, rcvr_ty, cal, span,
                     );
                 }
-                if let Some(span) = tcx.resolutions(()).confused_type_with_std_module.get(&span) {
+                if let Some(span) =
+                    tcx.resolutions(()).confused_type_with_std_module.get(&span.with_parent(None))
+                {
                     err.span_suggestion(
                         span.shrink_to_lo(),
                         "you are looking for the module in `std`, not the primitive type",

compiler/rustc_hir_typeck/src/op.rs

@@ -657,7 +657,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         }
                     }
 
-                    let sp = self.tcx.sess.source_map().start_point(ex.span);
+                    let sp = self.tcx.sess.source_map().start_point(ex.span).with_parent(None);
                     if let Some(sp) =
                         self.tcx.sess.parse_sess.ambiguous_block_expr_parse.borrow().get(&sp)
                     {

compiler/rustc_interface/src/callbacks.rs

@@ -10,6 +10,7 @@
 //! origin crate when the `TyCtxt` is not present in TLS.
 
 use rustc_errors::{Diagnostic, TRACK_DIAGNOSTICS};
+use rustc_middle::dep_graph::TaskDepsRef;
 use rustc_middle::ty::tls;
 use std::fmt;
 
@@ -26,14 +27,22 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
 /// This is a callback from `rustc_ast` as it cannot access the implicit state
 /// in `rustc_middle` otherwise. It is used when diagnostic messages are
 /// emitted and stores them in the current query, if there is one.
-fn track_diagnostic(diagnostic: &Diagnostic) {
+fn track_diagnostic(diagnostic: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnostic)) {
     tls::with_context_opt(|icx| {
         if let Some(icx) = icx {
             if let Some(diagnostics) = icx.diagnostics {
                 let mut diagnostics = diagnostics.lock();
                 diagnostics.extend(Some(diagnostic.clone()));
+                std::mem::drop(diagnostics);
             }
+
+            // Diagnostics are tracked, we can ignore the dependency.
+            let icx = tls::ImplicitCtxt { task_deps: TaskDepsRef::Ignore, ..icx.clone() };
+            return tls::enter_context(&icx, move |_| (*f)(diagnostic));
         }
+
+        // In any other case, invoke diagnostics anyway.
+        (*f)(diagnostic);
     })
 }
 
@@ -55,5 +64,5 @@ fn def_id_debug(def_id: rustc_hir::def_id::DefId, f: &mut fmt::Formatter<'_>) ->
 pub fn setup_callbacks() {
     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(&_)));
+    TRACK_DIAGNOSTICS.swap(&(track_diagnostic as _));
 }

compiler/rustc_interface/src/tests.rs

@@ -652,7 +652,6 @@ fn test_unstable_options_tracking_hash() {
     untracked!(future_incompat_test, true);
     untracked!(hir_stats, true);
     untracked!(identify_regions, true);
-    untracked!(incremental_ignore_spans, true);
     untracked!(incremental_info, true);
     untracked!(incremental_verify_ich, true);
     untracked!(input_stats, true);
@@ -737,6 +736,7 @@ fn test_unstable_options_tracking_hash() {
     tracked!(fuel, Some(("abc".to_string(), 99)));
     tracked!(function_sections, Some(false));
     tracked!(human_readable_cgu_names, true);
+    tracked!(incremental_ignore_spans, true);
     tracked!(inline_in_all_cgus, Some(true));
     tracked!(inline_mir, Some(true));
     tracked!(inline_mir_hint_threshold, Some(123));

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

@@ -1160,7 +1160,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
         hir_body_hash.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);
-        if tcx.sess.opts.unstable_opts.incremental_relative_spans {
+        if tcx.sess.opts.incremental_relative_spans() {
             let definitions = tcx.definitions_untracked();
             let mut owner_spans: Vec<_> = krate
                 .owners

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -634,7 +634,7 @@ impl<K: DepKind> DepGraph<K> {
         if dep_node_debug.borrow().contains_key(&dep_node) {
             return;
         }
-        let debug_str = debug_str_gen();
+        let debug_str = self.with_ignore(debug_str_gen);
         dep_node_debug.borrow_mut().insert(dep_node, debug_str);
     }
 
@@ -829,7 +829,9 @@ impl<K: DepKind> DepGraph<K> {
         );
 
         if !side_effects.is_empty() {
-            self.emit_side_effects(qcx, data, dep_node_index, side_effects);
+            self.with_query_deserialization(|| {
+                self.emit_side_effects(qcx, data, dep_node_index, side_effects)
+            });
         }
 
         // ... and finally storing a "Green" entry in the color map.

compiler/rustc_session/src/config.rs

@@ -787,6 +787,12 @@ impl Options {
     pub fn get_symbol_mangling_version(&self) -> SymbolManglingVersion {
         self.cg.symbol_mangling_version.unwrap_or(SymbolManglingVersion::Legacy)
     }
+
+    #[allow(rustc::bad_opt_access)]
+    pub fn incremental_relative_spans(&self) -> bool {
+        self.unstable_opts.incremental_relative_spans
+            || (self.unstable_features.is_nightly_build() && self.incremental.is_some())
+    }
 }
 
 impl UnstableOptions {

compiler/rustc_session/src/options.rs

@@ -1332,11 +1332,12 @@ options! {
         "generate human-readable, predictable names for codegen units (default: no)"),
     identify_regions: bool = (false, parse_bool, [UNTRACKED],
         "display unnamed regions as `'<id>`, using a non-ident unique id (default: no)"),
-    incremental_ignore_spans: bool = (false, parse_bool, [UNTRACKED],
+    incremental_ignore_spans: bool = (false, parse_bool, [TRACKED],
         "ignore spans during ICH computation -- used for testing (default: no)"),
     incremental_info: bool = (false, parse_bool, [UNTRACKED],
         "print high-level information about incremental reuse (or the lack thereof) \
         (default: no)"),
+    #[rustc_lint_opt_deny_field_access("use `Session::incremental_relative_spans` instead of this field")]
     incremental_relative_spans: bool = (false, parse_bool, [TRACKED],
         "hash spans relative to their parent item for incr. comp. (default: no)"),
     incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED],

compiler/rustc_span/src/hygiene.rs

@@ -104,6 +104,10 @@ fn assert_default_hashing_controls<CTX: HashStableContext>(ctx: &CTX, msg: &str)
         // `-Z incremental-ignore-spans` option. Normally, this option is disabled,
         // which will cause us to require that this method always be called with `Span` hashing
         // enabled.
+        //
+        // Span hashing can also be disabled without `-Z incremental-ignore-spans`.
+        // This is the case for instance when building a hash for name mangling.
+        // Such configuration must not be used for metadata.
         HashingControls { hash_spans }
             if hash_spans == !ctx.unstable_opts_incremental_ignore_spans() => {}
         other => panic!("Attempted hashing of {msg} with non-default HashingControls: {:?}", other),