Dubious improper_ctypes firing on recursive non-local containment?

[workingjubilee]

Bindgen allows you to create a "rustified non-exhaustive enum" from a C enum. This is used by pgrx-pg-sys for a specific C enum, NodeTag, because

• NodeTag has a well-defined, finite list of variants for each major version
• Despite this, much like a repr(Rust) enum, it regularly changes the assigned values to its variants each version, requiring an extension to be built against a specific Postgres version.
• Each major version consistently adds at least one variant somewhere.
• ...except for a zero-initialized value, which is always the sentinel value.
• ...so it is much better, in terms of ergonomics, to treat it as a Rust-ish enum with an appropriate repr to make sure it is the correct size, with #[non_exhaustive], so that people can write the same code and have it be robust against future Postgres versions (though I seriously doubt anyone is going to write a 400 variant match, but hey, we've seen weirder, sooo...).

Unfortunately, it seems this can trigger the improper_ctypes lint if people add their own bindings against Postgres which mention pgrx_pg_sys's bound types, which they did not define the original bindings to, and which only recursively contains the improper ctype... i.e. they mentioned a pgrx-pg-sys-bound type, which is itself a pointer to a pgrx-pg-sys-bound type, that contains a pointer to another pgrx-pg-sys-bound type that itself contains the pgrx-pg-sys-bound enum.

Naturally, they have no idea what the hell is going on, since the error doesn't even mention which field is the offender, or how recursively deep it is.

This disappears if I make it a "rustified enum" instead, which lacks #[non_exhaustive]. It seems some reasoning is being applied along the lines of, "Non-exhaustive means that it can become FFI-incompatible in the future due to feature(arbitrary_enum_discriminant) and feature(really_tagged_unions), without breaking compat", when that has nothing to do with reality, and in fact I'm more trying to use it to encourage people to handle the C code correctly! And even if that was the case, the lint appears to be violating locality.

I tried this code:

use pgrx::pg_sys;

//  Many
//  lines
//  of
//  boiler-
//  plate 
//  code.

extern "C" {
    pub fn rdb_read_page(
        index: pg_sys::Relation,
        blkno: u32,
        lock_type: ::std::os::raw::c_int,
        pg_buffer: *mut u32,
    ) -> *mut ::std::os::raw::c_char;
}

I got this:

warning: `extern` block uses type `NodeTag`, which is not FFI-safe
  --> src/lib.rs:39:16
   |
39 |         index: pg_sys::Relation,
   |                ^^^^^^^^^^^^^^^^ not FFI-safe
   |
   = note: this enum is non-exhaustive
   = note: `#[warn(improper_ctypes)]` on by default

warning: `sample` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 17.63s

I expected to not see a warning on types that weren't defined in the local crate, are de facto safe as argument types, and in any case are behind at least two pointers.

Meta

This repros on both stable and this nightly:

rustc --version --verbose:

rustc 1.75.0-nightly (fcab24817 2023-10-13)
binary: rustc
commit-hash: fcab24817c72ffbd6ffb66d92b7ddc0d3ee4d2f0
commit-date: 2023-10-13
host: x86_64-unknown-linux-gnu
release: 1.75.0-nightly
LLVM version: 17.0.2

[workingjubilee]

This was apparently decided thus by T-lang #44109 (comment) but I do not think that T-lang's reasoning holds up in the face of actual software composition and usage patterns.

[workingjubilee]

I should also note that removing non_exhaustive is actually really bad in my use-case: I need the non_exhaustive quality on the enum so programmers can't cast the enum to a random integer. People reaching for pgrx are often new to Rust, used to C, and thus will be flailing a bit, and if they start casting it to integers, they're about to shoot themselves in the foot if they make a comparison to integer literals, because it's not a stable value. This is the other half of why I wanted it as an enum.