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 25e2128
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 14 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,10 @@ 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::{
NonExhaustiveCLikeEnum, NonExhaustiveEnum, NonExhaustiveVariants,
NormalStruct, TupleStruct, UnitStruct,
};

extern "C" {
pub fn non_exhaustive_enum(_: NonExhaustiveEnum);
Expand All @@ -21,4 +24,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() {}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: `extern` block uses type `NonExhaustiveEnum`, which is not FFI-safe
--> $DIR/extern_crate_improper.rs:12:35
--> $DIR/extern_crate_improper.rs:15:35
|
LL | pub fn non_exhaustive_enum(_: NonExhaustiveEnum);
| ^^^^^^^^^^^^^^^^^ not FFI-safe
Expand All @@ -12,31 +12,31 @@ LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^

error: `extern` block uses type `NormalStruct`, which is not FFI-safe
--> $DIR/extern_crate_improper.rs:14:44
--> $DIR/extern_crate_improper.rs:17:44
|
LL | pub fn non_exhaustive_normal_struct(_: NormalStruct);
| ^^^^^^^^^^^^ not FFI-safe
|
= note: this struct is non-exhaustive

error: `extern` block uses type `UnitStruct`, which is not FFI-safe
--> $DIR/extern_crate_improper.rs:16:42
--> $DIR/extern_crate_improper.rs:19:42
|
LL | pub fn non_exhaustive_unit_struct(_: UnitStruct);
| ^^^^^^^^^^ not FFI-safe
|
= note: this struct is non-exhaustive

error: `extern` block uses type `TupleStruct`, which is not FFI-safe
--> $DIR/extern_crate_improper.rs:18:43
--> $DIR/extern_crate_improper.rs:21:43
|
LL | pub fn non_exhaustive_tuple_struct(_: TupleStruct);
| ^^^^^^^^^^^ not FFI-safe
|
= note: this struct is non-exhaustive

error: `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe
--> $DIR/extern_crate_improper.rs:20:38
--> $DIR/extern_crate_improper.rs:23:38
|
LL | pub fn non_exhaustive_variant(_: NonExhaustiveVariants);
| ^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
Expand Down

0 comments on commit 25e2128

Please sign in to comment.