Allow null-pointer-optimized enums in FFI if their underlying represe…

…ntation is FFI safe

This allows types like Option<NonZeroU8> to be used in FFI without triggering the improper_ctypes lint. This works by changing the is_repr_nullable_ptr function to consider an enum E to be FFI-safe if:

- E has no explicit #[repr(...)].
- It only has two variants.
- One of those variants is empty (meaning it has no fields).
- The other variant has only one field.
- That field is one of the following:
  - &T
  - &mut T
  - extern "C" fn
  - core::num::NonZero*
  - core::ptr::NonNull<T>
  - #[repr(transparent)] struct wrapper around one of the types in this list.
- The size of E and its field are both known and are both the same size (implying E is participating in the nonnull optimization).

src/libcore/num/mod.rs

@@ -50,6 +50,7 @@ assert_eq!(size_of::<Option<core::num::", stringify!($Ty), ">>(), size_of::<", s
  #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
  #[repr(transparent)]
  #[rustc_layout_scalar_valid_range_start(1)]
+ #[cfg_attr(not(stage0), rustc_nonnull_optimization_guaranteed)]
  pub struct $Ty($Int);
  }
 

src/libcore/ptr.rs

@@ -2938,6 +2938,7 @@ impl<'a, T: ?Sized> From<NonNull<T>> for Unique<T> {
 #[stable(feature = "nonnull", since = "1.25.0")]
 #[repr(transparent)]
 #[rustc_layout_scalar_valid_range_start(1)]
+#[cfg_attr(not(stage0), rustc_nonnull_optimization_guaranteed)]
 pub struct NonNull<T: ?Sized> {
  pointer: *const T,
 }

src/librustc_lint/types.rs

@@ -1,10 +1,11 @@
 #![allow(non_snake_case)]
 
 use rustc::hir::{ExprKind, Node};
+use crate::hir::def_id::DefId;
 use rustc::hir::lowering::is_range_literal;
 use rustc::ty::subst::SubstsRef;
 use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt};
-use rustc::ty::layout::{self, IntegerExt, LayoutOf, VariantIdx};
+use rustc::ty::layout::{self, IntegerExt, LayoutOf, VariantIdx, SizeSkeleton};
 use rustc::{lint, util};
 use rustc_data_structures::indexed_vec::Idx;
 use util::nodemap::FxHashSet;
@@ -14,11 +15,11 @@ use lint::{LintPass, LateLintPass};
 use std::cmp;
 use std::{i8, i16, i32, i64, u8, u16, u32, u64, f32, f64};
 
-use syntax::{ast, attr};
+use syntax::{ast, attr, source_map};
 use syntax::errors::Applicability;
+use syntax::symbol::sym;
 use rustc_target::spec::abi::Abi;
 use syntax_pos::Span;
-use syntax::source_map;
 
 use rustc::hir;
 
@@ -522,42 +523,79 @@ enum FfiResult<'tcx> {
  },
 }
 
+fn is_zst<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, did: DefId, ty: Ty<'tcx>) -> bool {
+ tcx.layout_of(tcx.param_env(did).and(ty)).map(|layout| layout.is_zst()).unwrap_or(false)
+}
+
+fn ty_is_known_nonnull<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, ty: Ty<'tcx>) -> bool {
+ match ty.sty {
+ ty::FnPtr(_) => true,
+ ty::Ref(..) => true,
+ ty::Adt(field_def, substs) if field_def.repr.transparent() && field_def.is_struct() => {
+ for field in &field_def.non_enum_variant().fields {
+ let field_ty = tcx.normalize_erasing_regions(
+ ParamEnv::reveal_all(),
+ field.ty(tcx, substs),
+ );
+ if is_zst(tcx, field.did, field_ty) {
+ continue;
+ }
+
+ let attrs = tcx.get_attrs(field_def.did);
+ if attrs.iter().any(|a| a.check_name(sym::rustc_nonnull_optimization_guaranteed)) ||
+ ty_is_known_nonnull(tcx, field_ty) {
+ return true;
+ }
+ }
+
+ false
+ }
+ _ => false,
+ }
+}
+
 /// Check if this enum can be safely exported based on the
 /// "nullable pointer optimization". Currently restricted
-/// to function pointers and references, but could be
-/// expanded to cover NonZero raw pointers and newtypes.
+/// to function pointers, references, core::num::NonZero*,
+/// core::ptr::NonNull, and #[repr(transparent)] newtypes.
 /// FIXME: This duplicates code in codegen.
 fn is_repr_nullable_ptr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
- def: &'tcx ty::AdtDef,
+ ty: Ty<'tcx>,
+ ty_def: &'tcx ty::AdtDef,
  substs: SubstsRef<'tcx>)
  -> bool {
- if def.variants.len() == 2 {
- let data_idx;
+ if ty_def.variants.len() != 2 {
+ return false;
+ }
 
- let zero = VariantIdx::new(0);
- let one = VariantIdx::new(1);
+ let get_variant_fields = |index| &ty_def.variants[VariantIdx::new(index)].fields;
+ let variant_fields = [get_variant_fields(0), get_variant_fields(1)];
+ let fields = if variant_fields[0].is_empty() {
+ &variant_fields[1]
+ } else if variant_fields[1].is_empty() {
+ &variant_fields[0]
+ } else {
+ return false;
+ };
 
- if def.variants[zero].fields.is_empty() {
- data_idx = one;
- } else if def.variants[one].fields.is_empty() {
- data_idx = zero;
- } else {
- return false;
- }
+ if fields.len() != 1 {
+ return false;
+ }
 
- if def.variants[data_idx].fields.len() == 1 {
- match def.variants[data_idx].fields[0].ty(tcx, substs).sty {
- ty::FnPtr(_) => {
- return true;
- }
- ty::Ref(..) => {
- return true;
- }
- _ => {}
- }
- }
+ let field_ty = fields[0].ty(tcx, substs);
+ if !ty_is_known_nonnull(tcx, field_ty) {
+ return false;
  }
- false
+
+ // At this point, the field's type is known to be nonnull and the parent enum is Option-like.
+ // If the computed size for the field and the enum are different, the nonnull optimization isn't
+ // being applied (and we've got a problem somewhere).
+ let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, ParamEnv::reveal_all()).unwrap();
+ if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) {
+ bug!("improper_ctypes: Option nonnull optimization not applied?");
+ }
+
+ true
 }
 
 impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
@@ -612,14 +650,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
  );
  // repr(transparent) types are allowed to have arbitrary ZSTs, not just
  // PhantomData -- skip checking all ZST fields
- if def.repr.transparent() {
- let is_zst = cx
- .layout_of(cx.param_env(field.did).and(field_ty))
- .map(|layout| layout.is_zst())
- .unwrap_or(false);
- if is_zst {
- continue;
- }
+ if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
+ continue;
  }
  let r = self.check_type_for_ffi(cache, field_ty);
  match r {
@@ -682,7 +714,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
  // discriminant.
  if !def.repr.c() && def.repr.int.is_none() {
  // Special-case types like `Option<extern fn()>`.
- if !is_repr_nullable_ptr(cx, def, substs) {
+ if !is_repr_nullable_ptr(cx, ty, def, substs) {
  return FfiUnsafe {
  ty: ty,
  reason: "enum has no representation hint",

src/libsyntax/feature_gate.rs

@@ -1134,6 +1134,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
  is just used to enable niche optimizations in libcore \
  and will never be stable",
  cfg_fn!(rustc_attrs))),
+ (sym::rustc_nonnull_optimization_guaranteed, Whitelisted, template!(Word),
+ Gated(Stability::Unstable,
+ sym::rustc_attrs,
+ "the `#[rustc_nonnull_optimization_guaranteed]` attribute \
+ is just used to enable niche optimizations in libcore \
+ and will never be stable",
+ cfg_fn!(rustc_attrs))),
  (sym::rustc_regions, Normal, template!(Word), Gated(Stability::Unstable,
  sym::rustc_attrs,
  "the `#[rustc_regions]` attribute \

src/libsyntax_pos/symbol.rs

@@ -491,6 +491,7 @@ symbols! {
  rustc_layout_scalar_valid_range_end,
  rustc_layout_scalar_valid_range_start,
  rustc_mir,
+ rustc_nonnull_optimization_guaranteed,
  rustc_object_lifetime_default,
  rustc_on_unimplemented,
  rustc_outlives,

src/test/ui/feature-gates/feature-gate-rustc-attrs-1.rs

@@ -4,5 +4,6 @@
 
 #[rustc_variance] //~ ERROR the `#[rustc_variance]` attribute is just used for rustc unit tests and will never be stable
 #[rustc_error] //~ ERROR the `#[rustc_error]` attribute is just used for rustc unit tests and will never be stable
+#[rustc_nonnull_optimization_guaranteed] //~ ERROR the `#[rustc_nonnull_optimization_guaranteed]` attribute is just used to enable niche optimizations in libcore and will never be stable
 
 fn main() {}

src/test/ui/feature-gates/feature-gate-rustc-attrs-1.stderr

@@ -16,6 +16,15 @@ LL | #[rustc_error]
  = note: for more information, see https://github.com/rust-lang/rust/issues/29642
  = help: add #![feature(rustc_attrs)] to the crate attributes to enable
 
-error: aborting due to 2 previous errors
+error[E0658]: the `#[rustc_nonnull_optimization_guaranteed]` attribute is just used to enable niche optimizations in libcore and will never be stable
+ --> $DIR/feature-gate-rustc-attrs-1.rs:7:1
+ |
+LL | #[rustc_nonnull_optimization_guaranteed]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: for more information, see https://github.com/rust-lang/rust/issues/29642
+ = help: add #![feature(rustc_attrs)] to the crate attributes to enable
+
+error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0658`.

src/test/ui/lint/lint-ctypes-enum.rs

@@ -1,6 +1,8 @@
 #![deny(improper_ctypes)]
 #![allow(dead_code)]
 
+use std::num;
+
 enum Z { }
 enum U { A }
 enum B { C, D }
@@ -15,14 +17,39 @@ enum U8 { A, B, C }
 #[repr(isize)]
 enum Isize { A, B, C }
 
+#[repr(transparent)]
+struct Transparent<T>(T, std::marker::PhantomData<Z>);
+
+struct Rust<T>(T);
+
 extern {
  fn zf(x: Z);
  fn uf(x: U); //~ ERROR enum has no representation hint
  fn bf(x: B); //~ ERROR enum has no representation hint
  fn tf(x: T); //~ ERROR enum has no representation hint
- fn reprc(x: ReprC);
- fn u8(x: U8);
- fn isize(x: Isize);
+ fn repr_c(x: ReprC);
+ fn repr_u8(x: U8);
+ fn repr_isize(x: Isize);
+ fn option_ref(x: Option<&'static u8>);
+ fn option_fn(x: Option<extern "C" fn()>);
+ fn nonnull(x: Option<std::ptr::NonNull<u8>>);
+ fn nonzero_u8(x: Option<num::NonZeroU8>);
+ fn nonzero_u16(x: Option<num::NonZeroU16>);
+ fn nonzero_u32(x: Option<num::NonZeroU32>);
+ fn nonzero_u64(x: Option<num::NonZeroU64>);
+ fn nonzero_u128(x: Option<num::NonZeroU128>);
+ //~^ ERROR 128-bit integers don't currently have a known stable ABI
+ fn nonzero_usize(x: Option<num::NonZeroUsize>);
+ fn nonzero_i8(x: Option<num::NonZeroI8>);
+ fn nonzero_i16(x: Option<num::NonZeroI16>);
+ fn nonzero_i32(x: Option<num::NonZeroI32>);
+ fn nonzero_i64(x: Option<num::NonZeroI64>);
+ fn nonzero_i128(x: Option<num::NonZeroI128>);
+ //~^ ERROR 128-bit integers don't currently have a known stable ABI
+ fn nonzero_isize(x: Option<num::NonZeroIsize>);
+ fn repr_transparent(x: Option<Transparent<num::NonZeroU8>>);
+ fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR enum has no representation hint
+ fn no_result(x: Result<(), num::NonZeroI32>); //~ ERROR enum has no representation hint
 }
 
 pub fn main() { }

src/test/ui/lint/lint-ctypes-enum.stderr

@@ -1,5 +1,5 @@
 error: `extern` block uses type `U` which is not FFI-safe: enum has no representation hint
- --> $DIR/lint-ctypes-enum.rs:20:13
+ --> $DIR/lint-ctypes-enum.rs:27:13
  |
 LL | fn uf(x: U);
  | ^
@@ -11,36 +11,64 @@ LL | #![deny(improper_ctypes)]
  | ^^^^^^^^^^^^^^^
  = help: consider adding a #[repr(...)] attribute to this enum
 note: type defined here
- --> $DIR/lint-ctypes-enum.rs:5:1
+ --> $DIR/lint-ctypes-enum.rs:7:1
  |
 LL | enum U { A }
  | ^^^^^^^^^^^^
 
 error: `extern` block uses type `B` which is not FFI-safe: enum has no representation hint
- --> $DIR/lint-ctypes-enum.rs:21:13
+ --> $DIR/lint-ctypes-enum.rs:28:13
  |
 LL | fn bf(x: B);
  | ^
  |
  = help: consider adding a #[repr(...)] attribute to this enum
 note: type defined here
- --> $DIR/lint-ctypes-enum.rs:6:1
+ --> $DIR/lint-ctypes-enum.rs:8:1
  |
 LL | enum B { C, D }
  | ^^^^^^^^^^^^^^^
 
 error: `extern` block uses type `T` which is not FFI-safe: enum has no representation hint
- --> $DIR/lint-ctypes-enum.rs:22:13
+ --> $DIR/lint-ctypes-enum.rs:29:13
  |
 LL | fn tf(x: T);
  | ^
  |
  = help: consider adding a #[repr(...)] attribute to this enum
 note: type defined here
- --> $DIR/lint-ctypes-enum.rs:7:1
+ --> $DIR/lint-ctypes-enum.rs:9:1
  |
 LL | enum T { E, F, G }
  | ^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 3 previous errors
+error: `extern` block uses type `u128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI
+ --> $DIR/lint-ctypes-enum.rs:40:23
+ |
+LL | fn nonzero_u128(x: Option<num::NonZeroU128>);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: `extern` block uses type `i128` which is not FFI-safe: 128-bit integers don't currently have a known stable ABI
+ --> $DIR/lint-ctypes-enum.rs:47:23
+ |
+LL | fn nonzero_i128(x: Option<num::NonZeroI128>);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: `extern` block uses type `std::option::Option<Rust<std::num::NonZeroU8>>` which is not FFI-safe: enum has no representation hint
+ --> $DIR/lint-ctypes-enum.rs:51:20
+ |
+LL | fn repr_rust(x: Option<Rust<num::NonZeroU8>>);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: consider adding a #[repr(...)] attribute to this enum
+
+error: `extern` block uses type `std::result::Result<(), std::num::NonZeroI32>` which is not FFI-safe: enum has no representation hint
+ --> $DIR/lint-ctypes-enum.rs:52:20
+ |
+LL | fn no_result(x: Result<(), num::NonZeroI32>);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: consider adding a #[repr(...)] attribute to this enum
+
+error: aborting due to 7 previous errors