Rollup merge of #67054 - RalfJung:set-discriminant-unreachable, r=oli-obk

codegen "unreachable" for invalid SetDiscriminant

Follow-up from #66960. I also realized I don't understand our policy for using `abort` vs `unreachable`. AFAIK `abort` is safe to call and just aborts the process, while `unreachable` is UB. But sometimes we use both, like here

https://github.com/rust-lang/rust/blob/d825e35ee8325146e6c175a4c61bcb645b347d5e/src/librustc_codegen_ssa/mir/block.rs#L827-L828

and here

https://github.com/rust-lang/rust/blob/d825e35ee8325146e6c175a4c61bcb645b347d5e/src/librustc_codegen_ssa/mir/block.rs#L264-L265

The second case is even more confusing because that looks like an unreachable `return` to me, so why would we codegen a safe abort there?

r? @eddyb Cc @oli-obk

Author: JohnTitor

src/librustc_codegen_ssa/mir/block.rs

@@ -261,7 +261,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         if self.fn_abi.ret.layout.abi.is_uninhabited() {
             // Functions with uninhabited return values are marked `noreturn`,
             // so we should make sure that we never actually do.
+            // We play it safe by using a well-defined `abort`, but we could go for immediate UB
+            // if that turns out to be helpful.
             bx.abort();
+            // `abort` does not terminate the block, so we still need to generate
+            // an `unreachable` terminator after it.
             bx.unreachable();
             return;
         }
@@ -825,6 +829,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
             mir::TerminatorKind::Abort => {
                 bx.abort();
+                // `abort` does not terminate the block, so we still need to generate
+                // an `unreachable` terminator after it.
                 bx.unreachable();
             }
 

src/librustc_codegen_ssa/mir/operand.rs

@@ -475,9 +475,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             },
                         }
                         // Allow RalfJ to sleep soundly knowing that even refactorings that remove
-                        // the above error (or silence it under some conditions) will not cause UB
+                        // the above error (or silence it under some conditions) will not cause UB.
                         bx.abort();
-                        // We've errored, so we don't have to produce working code.
+                        // We still have to return an operand but it doesn't matter,
+                        // this code is unreachable.
                         let ty = self.monomorphize(&constant.literal.ty);
                         let layout = bx.cx().layout_of(ty);
                         bx.load_operand(PlaceRef::new_sized(

src/librustc_codegen_ssa/mir/place.rs

@@ -333,6 +333,9 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         variant_index: VariantIdx
     ) {
         if self.layout.for_variant(bx.cx(), variant_index).abi.is_uninhabited() {
+            // We play it safe by using a well-defined `abort`, but we could go for immediate UB
+            // if that turns out to be helpful.
+            bx.abort();
             return;
         }
         match self.layout.variants {
@@ -488,10 +491,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                     },
                     Err(_) => {
                         // This is unreachable as long as runtime
-                        // and compile-time agree on values
+                        // and compile-time agree perfectly.
                         // With floats that won't always be true,
-                        // so we generate an abort.
+                        // so we generate a (safe) abort.
                         bx.abort();
+                        // We still have to return a place but it doesn't matter,
+                        // this code is unreachable.
                         let llval = bx.cx().const_undef(
                             bx.cx().type_ptr_to(bx.cx().backend_type(layout))
                         );

src/test/codegen/set-discriminant-invalid.rs

@@ -0,0 +1,43 @@
+// compile-flags: -C opt-level=0
+#![crate_type = "lib"]
+
+pub enum ApiError {}
+#[allow(dead_code)]
+pub struct TokioError {
+    b: bool,
+}
+pub enum Error {
+    Api {
+        source: ApiError,
+    },
+    Ethereum,
+    Tokio {
+        source: TokioError,
+    },
+}
+struct Api;
+impl IntoError<Error> for Api
+{
+    type Source = ApiError;
+    // CHECK-LABEL: @into_error
+    // CHECK: llvm.trap()
+    // Also check the next two instructions to make sure we do not match against `trap`
+    // elsewhere in the code.
+    // CHECK-NEXT: load
+    // CHECK-NEXT: ret
+    #[no_mangle]
+    fn into_error(self, error: Self::Source) -> Error {
+        Error::Api {
+            source: (|v| v)(error),
+        }
+    }
+}
+
+pub trait IntoError<E>
+{
+    /// The underlying error
+    type Source;
+
+    /// Combine the information to produce the error
+    fn into_error(self, source: Self::Source) -> E;
+}