Always register zombie messages, only at most defer their Spans.

Author: eddyb

crates/rustc_codegen_spirv/src/builder/builder_methods.rs

@@ -2218,11 +2218,25 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
             );
             // Defer the cast so that it has a chance to be avoided.
             let original_ptr = ptr.def(self);
+            let bitcast_result_id = self.emit().bitcast(dest_ty, None, original_ptr).unwrap();
+
+            self.zombie(
+                bitcast_result_id,
+                &format!(
+                    "cannot cast between pointer types\
+                         \nfrom `{}`\
+                         \n  to `{}`",
+                    self.debug_type(ptr.ty),
+                    self.debug_type(dest_ty)
+                ),
+            );
+
             SpirvValue {
+                zombie_waiting_for_span: false,
                 kind: SpirvValueKind::LogicalPtrCast {
                     original_ptr,
                     original_ptr_ty: ptr.ty,
-                    bitcast_result_id: self.emit().bitcast(dest_ty, None, original_ptr).unwrap(),
+                    bitcast_result_id,
                 },
                 ty: dest_ty,
             }

crates/rustc_codegen_spirv/src/builder_spirv.rs

@@ -70,6 +70,13 @@ pub enum SpirvValueKind {
 
 #[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)]
 pub struct SpirvValue {
+    // HACK(eddyb) used to cheaply check whether this is a SPIR-V value ID
+    // with a "zombie" (deferred error) attached to it, that may need a `Span`
+    // still (e.g. such as constants, which can't easily take a `Span`).
+    // FIXME(eddyb) a whole `bool` field is sadly inefficient, but anything
+    // which may make `SpirvValue` smaller requires far too much impl effort.
+    pub zombie_waiting_for_span: bool,
+
     pub kind: SpirvValueKind,
     pub ty: Word,
 }
@@ -103,7 +110,11 @@ impl SpirvValue {
                         } else {
                             SpirvValueKind::IllegalConst(pointee)
                         };
-                        Some(SpirvValue { kind, ty })
+                        Some(SpirvValue {
+                            zombie_waiting_for_span: entry.legal.is_err(),
+                            kind,
+                            ty,
+                        })
                     }
                     _ => None,
                 }
@@ -127,38 +138,7 @@ impl SpirvValue {
     }
 
     pub fn def_with_span(self, cx: &CodegenCx<'_>, span: Span) -> Word {
-        match self.kind {
-            SpirvValueKind::Def(id) => id,
-
-            SpirvValueKind::IllegalConst(id) => {
-                let entry = &cx.builder.id_to_const.borrow()[&id];
-                let msg = match entry.legal.unwrap_err() {
-                    IllegalConst::Shallow(cause) => {
-                        if let (
-                            LeafIllegalConst::CompositeContainsPtrTo,
-                            SpirvConst::Composite(_fields),
-                        ) = (cause, &entry.val)
-                        {
-                            // FIXME(eddyb) materialize this at runtime, using
-                            // `OpCompositeConstruct` (transitively, i.e. after
-                            // putting every field through `SpirvValue::def`),
-                            // if we have a `Builder` to do that in.
-                            // FIXME(eddyb) this isn't possible right now, as
-                            // the builder would be dynamically "locked" anyway
-                            // (i.e. attempting to do `bx.emit()` would panic).
-                        }
-
-                        cause.message()
-                    }
-
-                    IllegalConst::Indirect(cause) => cause.message(),
-                };
-
-                cx.zombie_with_span(id, span, msg);
-
-                id
-            }
-
+        let id = match self.kind {
             SpirvValueKind::FnAddr { .. } => {
                 cx.builder
                     .const_to_id
@@ -171,26 +151,18 @@ impl SpirvValue {
                     .val
             }
 
-            SpirvValueKind::LogicalPtrCast {
+            SpirvValueKind::Def(id)
+            | SpirvValueKind::IllegalConst(id)
+            | SpirvValueKind::LogicalPtrCast {
                 original_ptr: _,
-                original_ptr_ty,
-                bitcast_result_id,
-            } => {
-                cx.zombie_with_span(
-                    bitcast_result_id,
-                    span,
-                    &format!(
-                        "cannot cast between pointer types\
-                         \nfrom `{}`\
-                         \n  to `{}`",
-                        cx.debug_type(original_ptr_ty),
-                        cx.debug_type(self.ty)
-                    ),
-                );
-
-                bitcast_result_id
-            }
+                original_ptr_ty: _,
+                bitcast_result_id: id,
+            } => id,
+        };
+        if self.zombie_waiting_for_span {
+            cx.add_span_to_zombie_if_missing(id, span);
         }
+        id
     }
 }
 
@@ -201,6 +173,7 @@ pub trait SpirvValueExt {
 impl SpirvValueExt for Word {
     fn with_type(self, ty: Word) -> SpirvValue {
         SpirvValue {
+            zombie_waiting_for_span: false,
             kind: SpirvValueKind::Def(self),
             ty,
         }
@@ -606,7 +579,11 @@ impl<'tcx> BuilderSpirv<'tcx> {
             } else {
                 SpirvValueKind::IllegalConst(entry.val)
             };
-            return SpirvValue { kind, ty };
+            return SpirvValue {
+                zombie_waiting_for_span: entry.legal.is_err(),
+                kind,
+                ty,
+            };
         }
         let val = val_with_type.val;
 
@@ -783,6 +760,17 @@ impl<'tcx> BuilderSpirv<'tcx> {
                 LeafIllegalConst::UntypedConstDataFromAlloc,
             )),
         };
+
+        // FIXME(eddyb) avoid dragging "const (il)legality" around, as well
+        // (sadly that does require that `SpirvConst` -> SPIR-V be injective,
+        // e.g. `OpUndef` can never be used for unrepresentable constants).
+        if let Err(illegal) = legal {
+            let msg = match illegal {
+                IllegalConst::Shallow(cause) | IllegalConst::Indirect(cause) => cause.message(),
+            };
+            cx.zombie_no_span(id, msg);
+        }
+
         let val = val.tcx_arena_alloc_slices(cx);
         assert_matches!(
             self.const_to_id
@@ -802,7 +790,11 @@ impl<'tcx> BuilderSpirv<'tcx> {
         } else {
             SpirvValueKind::IllegalConst(id)
         };
-        SpirvValue { kind, ty }
+        SpirvValue {
+            zombie_waiting_for_span: legal.is_err(),
+            kind,
+            ty,
+        }
     }
 
     pub fn lookup_const_by_id(&self, id: Word) -> Option<SpirvConst<'tcx, 'tcx>> {

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

@@ -245,9 +245,9 @@ impl<'tcx> CodegenCx<'tcx> {
     /// is stripped from the binary.
     ///
     /// Errors will only be emitted (by `linker::zombies`) for reachable zombies.
-    pub fn zombie_with_span(&self, word: Word, span: Span, reason: &str) {
+    pub fn zombie_with_span(&self, id: Word, span: Span, reason: &str) {
         self.zombie_decorations.borrow_mut().insert(
-            word,
+            id,
             (
                 ZombieDecoration {
                     // FIXME(eddyb) this could take advantage of `Cow` and use
@@ -258,8 +258,16 @@ impl<'tcx> CodegenCx<'tcx> {
             ),
         );
     }
-    pub fn zombie_no_span(&self, word: Word, reason: &str) {
-        self.zombie_with_span(word, DUMMY_SP, reason);
+    pub fn zombie_no_span(&self, id: Word, reason: &str) {
+        self.zombie_with_span(id, DUMMY_SP, reason);
+    }
+
+    pub fn add_span_to_zombie_if_missing(&self, id: Word, span: Span) {
+        if span != DUMMY_SP
+            && let Some((_, src_loc @ None)) = self.zombie_decorations.borrow_mut().get_mut(&id)
+        {
+            *src_loc = SrcLocDecoration::from_rustc_span(span, &self.builder);
+        }
     }
 
     pub fn finalize_module(self) -> Module {
@@ -851,6 +859,7 @@ impl<'tcx> MiscCodegenMethods<'tcx> for CodegenCx<'tcx> {
         self.def_constant(ty, SpirvConst::ZombieUndefForFnAddr);
 
         SpirvValue {
+            zombie_waiting_for_span: false,
             kind: SpirvValueKind::FnAddr {
                 function: function.id,
             },

tests/compiletests/ui/dis/ptr_copy.normal.stderr

@@ -28,6 +28,12 @@ note: called by `main`
 error: cannot cast between pointer types
        from `*f32`
          to `*struct () {  }`
+   --> $CORE_SRC/ptr/mod.rs:625:34
+    |
+625 |                 src: *const () = src as *const (),
+    |                                  ^^^^^^^^^^^^^^^^
+    |
+note: used from within `core::ptr::copy::<f32>`
    --> $CORE_SRC/ptr/mod.rs:621:9
     |
 621 | /         ub_checks::assert_unsafe_precondition!(
@@ -37,6 +43,29 @@ error: cannot cast between pointer types
 631 | |                 && ub_checks::maybe_is_aligned_and_not_null(dst, align, zero_size)
 632 | |         );
     | |_________^
+note: called by `ptr_copy::copy_via_raw_ptr`
+   --> $DIR/ptr_copy.rs:28:18
+    |
+28  |         unsafe { core::ptr::copy(src, dst, 1) }
+    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: called by `ptr_copy::main`
+   --> $DIR/ptr_copy.rs:33:5
+    |
+33  |     copy_via_raw_ptr(&i, o);
+    |     ^^^^^^^^^^^^^^^^^^^^^^^
+note: called by `main`
+   --> $DIR/ptr_copy.rs:32:8
+    |
+32  | pub fn main(i: f32, o: &mut f32) {
+    |        ^^^^
+
+error: cannot cast between pointer types
+       from `*f32`
+         to `*struct () {  }`
+   --> $CORE_SRC/ptr/mod.rs:626:32
+    |
+626 |                 dst: *mut () = dst as *mut (),
+    |                                ^^^^^^^^^^^^^^
     |
 note: used from within `core::ptr::copy::<f32>`
    --> $CORE_SRC/ptr/mod.rs:621:9
@@ -64,5 +93,5 @@ note: called by `main`
 32  | pub fn main(i: f32, o: &mut f32) {
     |        ^^^^
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors