Skip to content

Commit

Permalink
warn less about non-exhaustive in ffi
Browse files Browse the repository at this point in the history
Bindgen allows generating `#[non_exhaustive] #[repr(u32)]` enums.
This results in nonintuitive nonlocal `improper_ctypes` warnings,
even when the types are otherwise perfectly valid in C.

Adjust for actual tooling expectations by avoiding warning on
simple enums with only unit variants.
  • Loading branch information
workingjubilee committed Oct 17, 2023
1 parent 631a116 commit 2ce1399
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
25 changes: 17 additions & 8 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_attr as attr;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::DiagnosticMessage;
use rustc_hir as hir;
use rustc_hir::{is_range_literal, Expr, ExprKind, Node};
use rustc_hir::{def::CtorKind, is_range_literal, Expr, ExprKind, Node};
use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton};
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{
Expand Down Expand Up @@ -1136,16 +1136,25 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}

if def.is_variant_list_non_exhaustive() && !def.did().is_local() {
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_non_exhaustive,
help: None,
};
}
// non_exhaustive suggests it is possible that someone might break ABI
// see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344
// so warn on complex enums being used outside their crate
let nonexhaustive_nonlocal_ffi =
def.is_variant_list_non_exhaustive() && !def.did().is_local();

// Check the contained variants.
for variant in def.variants() {
// but only warn about really_tagged_union reprs,
// exempt enums with unit ctors like C's (like rust-bindgen)
if nonexhaustive_nonlocal_ffi
&& !matches!(variant.ctor_kind(), Some(CtorKind::Const))
{
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_non_exhaustive,
help: None,
};
};
let is_non_exhaustive = variant.is_field_list_non_exhaustive();
if is_non_exhaustive && !variant.def_id.is_local() {
return FfiUnsafe {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,14 @@ pub enum NonExhaustiveVariants {
#[non_exhaustive] Tuple(u32),
#[non_exhaustive] Struct { field: u32 }
}

// Note the absence of repr(C): it's not necessary, and recent C code can now use repr hints too.
#[repr(u32)]
#[non_exhaustive]
pub enum NonExhaustiveCLikeEnum {
One = 1,
Two = 2,
Three = 3,
Four = 4,
Five = 5,
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ extern crate types;
// This test checks that non-exhaustive types with `#[repr(C)]` from an extern crate are considered
// improper.

use types::{NonExhaustiveEnum, NonExhaustiveVariants, NormalStruct, TupleStruct, UnitStruct};
use types::{NonExhaustiveEnum, NonExhaustiveCLikeEnum, NonExhaustiveVariants, NormalStruct, TupleStruct, UnitStruct};

Check failure on line 9 in tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars

extern "C" {
pub fn non_exhaustive_enum(_: NonExhaustiveEnum);
Expand All @@ -21,4 +21,9 @@ extern "C" {
//~^ ERROR `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe
}

// These should pass without remark, as they're C-compatible, despite being "non-exhaustive".
extern "C" {
pub fn non_exhaustive_c_compat_enum(_: NonExhaustiveCLikeEnum);
}

fn main() {}

0 comments on commit 2ce1399

Please sign in to comment.