Auto merge of #148863 - jdonszelmann:check-span-owners, r=<try>

Check span owners (span lowering) in debug builds and fix missing lowerings

Author: rust-bors[bot]

compiler/rustc_ast_lowering/src/asm.rs

@@ -496,7 +496,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
 
         let operands = self.arena.alloc_from_iter(operands);
-        let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
+        let template = self
+            .arena
+            .alloc_from_iter(asm.template.iter().map(|i| self.lower_inline_asm_template_piece(i)));
         let template_strs = self.arena.alloc_from_iter(
             asm.template_strs
                 .iter()
@@ -514,4 +516,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         };
         self.arena.alloc(hir_asm)
     }
+
+    fn lower_inline_asm_template_piece(
+        &self,
+        template: &InlineAsmTemplatePiece,
+    ) -> InlineAsmTemplatePiece {
+        match template {
+            InlineAsmTemplatePiece::String(cow) => InlineAsmTemplatePiece::String(cow.clone()),
+            InlineAsmTemplatePiece::Placeholder { operand_idx, modifier, span } => {
+                InlineAsmTemplatePiece::Placeholder {
+                    operand_idx: *operand_idx,
+                    modifier: *modifier,
+                    span: self.lower_span(*span),
+                }
+            }
+        }
+    }
 }

compiler/rustc_ast_lowering/src/item.rs

@@ -555,6 +555,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     // own its own names, we have to adjust the owner before
                     // lowering the rest of the import.
                     self.with_hir_id_owner(id, |this| {
+                        let vis_span = this.lower_span(vis_span);
+
                         // `prefix` is lowered multiple times, but in different HIR owners.
                         // So each segment gets renewed `HirId` with the same
                         // `ItemLocalId` and the new owner. (See `lower_node_id`)
@@ -570,6 +572,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             span: this.lower_span(use_tree.span),
                             has_delayed_lints: !this.delayed_lints.is_empty(),
                         };
+
                         hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }

compiler/rustc_ast_lowering/src/lib.rs

@@ -38,6 +38,8 @@
 use std::sync::Arc;
 
 use rustc_ast::node_id::NodeMap;
+use rustc_ast::token::Token;
+use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
 use rustc_ast::{self as ast, *};
 use rustc_attr_parsing::{AttributeParser, Late, OmitDoc};
 use rustc_data_structures::fingerprint::Fingerprint;
@@ -1007,8 +1009,36 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    fn lower_delim_args(&self, args: &DelimArgs) -> DelimArgs {
-        args.clone()
+    fn lower_token_tree(&self, tt: &TokenTree) -> TokenTree {
+        match tt {
+            TokenTree::Token(Token { kind, span }, spacing) => {
+                TokenTree::Token(Token { kind: *kind, span: self.lower_span(*span) }, *spacing)
+            }
+            TokenTree::Delimited(delim_span, delim_spacing, delimiter, token_stream) => {
+                TokenTree::Delimited(
+                    self.lower_delim_span(delim_span),
+                    *delim_spacing,
+                    *delimiter,
+                    self.lower_token_stream(token_stream),
+                )
+            }
+        }
+    }
+
+    fn lower_token_stream(&self, ts: &TokenStream) -> TokenStream {
+        TokenStream::new(ts.iter().map(|i| self.lower_token_tree(i)).collect())
+    }
+
+    fn lower_delim_span(&self, DelimSpan { open, close }: &DelimSpan) -> DelimSpan {
+        DelimSpan { open: self.lower_span(*open), close: self.lower_span(*close) }
+    }
+
+    fn lower_delim_args(&self, DelimArgs { dspan, delim, tokens }: &DelimArgs) -> DelimArgs {
+        DelimArgs {
+            dspan: self.lower_delim_span(dspan),
+            delim: *delim,
+            tokens: self.lower_token_stream(tokens),
+        }
     }
 
     /// Lower an associated item constraint.

compiler/rustc_hir/src/stable_hash_impls.rs

@@ -1,5 +1,7 @@
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
 use rustc_span::def_id::DefPathHash;
+#[cfg(debug_assertions)]
+use rustc_span::def_id::LocalDefId;
 
 use crate::HashIgnoredAttrId;
 use crate::hir::{
@@ -13,6 +15,9 @@ use crate::lints::DelayedLints;
 /// instead of implementing everything in `rustc_middle`.
 pub trait HashStableContext: rustc_ast::HashStableContext + rustc_abi::HashStableContext {
     fn hash_attr_id(&mut self, id: &HashIgnoredAttrId, hasher: &mut StableHasher);
+
+    #[cfg(debug_assertions)]
+    fn set_current_owner_node_defid(&mut self, local_def_id: Option<LocalDefId>);
 }
 
 impl<HirCtx: crate::HashStableContext> ToStableHashKey<HirCtx> for BodyId {

compiler/rustc_middle/src/hir/mod.rs

@@ -177,12 +177,27 @@ impl<'tcx> TyCtxt<'tcx> {
         }
 
         self.with_stable_hashing_context(|mut hcx| {
+            // With debug assertions on, *while hashing*,
+            // we will check whether spans properly set their parent to the current node's defid
+            // For that we set the defid here to check against
+            #[cfg(debug_assertions)]
+            if self.sess.opts.incremental.is_some() && !matches!(node, OwnerNode::Synthetic) {
+                hcx.set_current_owner_node_defid(Some(node.def_id().def_id));
+            }
+
             let mut stable_hasher = StableHasher::new();
             node.hash_stable(&mut hcx, &mut stable_hasher);
             // Bodies are stored out of line, so we need to pull them explicitly in the hash.
             bodies.hash_stable(&mut hcx, &mut stable_hasher);
             let h1 = stable_hasher.finish();
 
+            // At the start of hashing an owner node we set this to the node's defid.
+            // We clear it again here, ending checking of spans.
+            #[cfg(debug_assertions)]
+            if self.sess.opts.incremental.is_some() {
+                hcx.set_current_owner_node_defid(None);
+            }
+
             let mut stable_hasher = StableHasher::new();
             attrs.hash_stable(&mut hcx, &mut stable_hasher);
 

compiler/rustc_query_system/src/ich/hcx.rs

@@ -21,6 +21,10 @@ pub struct StableHashingContext<'a> {
     // The value of `-Z incremental-ignore-spans`.
     // This field should only be used by `unstable_opts_incremental_ignore_span`
     incremental_ignore_spans: bool,
+
+    #[cfg(debug_assertions)]
+    pub(crate) current_owner_node_did: Option<LocalDefId>,
+
     // Very often, we are hashing something that does not need the
     // `CachingSourceMapView`, so we initialize it lazily.
     raw_source_map: &'a SourceMap,
@@ -36,6 +40,8 @@ impl<'a> StableHashingContext<'a> {
         StableHashingContext {
             untracked,
             incremental_ignore_spans: sess.opts.unstable_opts.incremental_ignore_spans,
+            #[cfg(debug_assertions)]
+            current_owner_node_did: None,
             caching_source_map: None,
             raw_source_map: sess.source_map(),
             hashing_controls: HashingControls { hash_spans: hash_spans_initial },
@@ -126,6 +132,26 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
     fn hashing_controls(&self) -> HashingControls {
         self.hashing_controls.clone()
     }
+
+    #[cfg(debug_assertions)]
+    fn validate_span_parent(&self, span: Span) {
+        // If we're hashing an owner node right now...
+        let Some(current_owner_node_did) = self.current_owner_node_did else {
+            return;
+        };
+
+        // we don't care about the dummy span
+        if span.is_dummy() {
+            return;
+        }
+
+        // then the parent must be set and match that defid.
+        assert!(
+            span.parent().is_some_and(|i| i == current_owner_node_did),
+            "Expected span to be lowered. Lowered spans have their parent set to their enclosing owner node. However, contained in the owner node with defid={current_owner_node_did:?} a span={span:?} was found whose parent is {:?}.",
+            span.parent()
+        )
+    }
 }
 
 impl<'a> rustc_session::HashStableContext for StableHashingContext<'a> {}

compiler/rustc_query_system/src/ich/impls_syntax.rs

@@ -2,6 +2,8 @@
 //! from various crates in no particular order.
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+#[cfg(debug_assertions)]
+use rustc_hir::def_id::LocalDefId;
 use rustc_hir::{self as hir, HashIgnoredAttrId};
 use rustc_span::SourceFile;
 use smallvec::SmallVec;
@@ -39,6 +41,15 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
     fn hash_attr_id(&mut self, _id: &HashIgnoredAttrId, _hasher: &mut StableHasher) {
         /* we don't hash HashIgnoredAttrId, we ignore them */
     }
+
+    #[cfg(debug_assertions)]
+    fn set_current_owner_node_defid(&mut self, local_def_id: Option<LocalDefId>) {
+        if local_def_id.is_some() && self.current_owner_node_did.is_some() {
+            panic!("already in owner node");
+        }
+
+        self.current_owner_node_did = local_def_id;
+    }
 }
 
 impl<'a> HashStable<StableHashingContext<'a>> for SourceFile {

compiler/rustc_span/src/lib.rs

@@ -2634,6 +2634,9 @@ pub trait HashStableContext {
         span: &SpanData,
     ) -> Option<(StableSourceFileId, usize, BytePos, usize, BytePos)>;
     fn hashing_controls(&self) -> HashingControls;
+
+    #[cfg(debug_assertions)]
+    fn validate_span_parent(&self, span: Span);
 }
 
 impl<CTX> HashStable<CTX> for Span
@@ -2663,6 +2666,12 @@ where
         span.ctxt.hash_stable(ctx, hasher);
         span.parent.hash_stable(ctx, hasher);
 
+        // check whether the span is lowered correctly when hashing
+        #[cfg(debug_assertions)]
+        {
+            ctx.validate_span_parent(*self);
+        }
+
         if span.is_dummy() {
             Hash::hash(&TAG_INVALID_SPAN, hasher);
             return;