From f441adc89a880df995ed95ca1402cfe8939d273b Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 14:35:16 -0700 Subject: [PATCH 1/2] Generate safe stable code for derives on empty enums Generate `match *self {}` instead of `unsafe { core::intrinsics::unreachable() }`. This is: 1. safe 2. stable for the benefit of everyone looking at these derived impls through `cargo expand`. Both expansions compile to the same code at all optimization levels (including `0`). --- .../src/deriving/generic/mod.rs | 17 +++++++++++++---- tests/ui/deriving/deriving-all-codegen.stdout | 14 +++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 4ba09335cb7..0f51b26de09 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1134,9 +1134,14 @@ impl<'a> MethodDef<'a> { trait_: &TraitDef<'b>, enum_def: &'b EnumDef, type_ident: Ident, - selflike_args: ThinVec>, + mut selflike_args: ThinVec>, nonselflike_args: &[P], ) -> BlockOrExpr { + assert!( + !selflike_args.is_empty(), + "static methods must use `expand_static_enum_method_body`", + ); + let span = trait_.span; let variants = &enum_def.variants; @@ -1144,10 +1149,14 @@ impl<'a> MethodDef<'a> { let unify_fieldless_variants = self.fieldless_variants_strategy == FieldlessVariantsStrategy::Unify; - // There is no sensible code to be generated for *any* deriving on a - // zero-variant enum. So we just generate a failing expression. + // For zero-variant enum, this function body is unreachable. + // Generate `match *self {}`. if variants.is_empty() { - return BlockOrExpr(ThinVec::new(), Some(deriving::call_unreachable(cx, span))); + selflike_args.truncate(1); + let match_arg = cx.expr_deref(span, selflike_args.pop().unwrap()); + let match_arms = ThinVec::new(); + let expr = cx.expr_match(span, match_arg, match_arms); + return BlockOrExpr(ThinVec::new(), Some(expr)); } let prefixes = iter::once("__self".to_string()) diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout index d6a2c80cc06..6bfc859bfe8 100644 --- a/tests/ui/deriving/deriving-all-codegen.stdout +++ b/tests/ui/deriving/deriving-all-codegen.stdout @@ -798,14 +798,14 @@ impl ::core::marker::Copy for Enum0 { } #[automatically_derived] impl ::core::fmt::Debug for Enum0 { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - unsafe { ::core::intrinsics::unreachable() } + match *self {} } } #[automatically_derived] impl ::core::hash::Hash for Enum0 { #[inline] fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () { - unsafe { ::core::intrinsics::unreachable() } + match *self {} } } #[automatically_derived] @@ -813,9 +813,7 @@ impl ::core::marker::StructuralPartialEq for Enum0 { } #[automatically_derived] impl ::core::cmp::PartialEq for Enum0 { #[inline] - fn eq(&self, other: &Enum0) -> bool { - unsafe { ::core::intrinsics::unreachable() } - } + fn eq(&self, other: &Enum0) -> bool { match *self {} } } #[automatically_derived] impl ::core::marker::StructuralEq for Enum0 { } @@ -831,15 +829,13 @@ impl ::core::cmp::PartialOrd for Enum0 { #[inline] fn partial_cmp(&self, other: &Enum0) -> ::core::option::Option<::core::cmp::Ordering> { - unsafe { ::core::intrinsics::unreachable() } + match *self {} } } #[automatically_derived] impl ::core::cmp::Ord for Enum0 { #[inline] - fn cmp(&self, other: &Enum0) -> ::core::cmp::Ordering { - unsafe { ::core::intrinsics::unreachable() } - } + fn cmp(&self, other: &Enum0) -> ::core::cmp::Ordering { match *self {} } } // A single-variant enum. From 56633b3f51967f544c988dcabbcd1b99f4cc3558 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 16 Jul 2023 15:33:29 -0700 Subject: [PATCH 2/2] Add a comparison between match *self and intrinsics::unreachable() --- compiler/rustc_builtin_macros/src/deriving/generic/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 0f51b26de09..9865b6a72ee 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1149,8 +1149,9 @@ impl<'a> MethodDef<'a> { let unify_fieldless_variants = self.fieldless_variants_strategy == FieldlessVariantsStrategy::Unify; - // For zero-variant enum, this function body is unreachable. - // Generate `match *self {}`. + // For zero-variant enum, this function body is unreachable. Generate + // `match *self {}`. This produces machine code identical to `unsafe { + // core::intrinsics::unreachable() }` while being safe and stable. if variants.is_empty() { selflike_args.truncate(1); let match_arg = cx.expr_deref(span, selflike_args.pop().unwrap());