(
hi Jubilee, I'm back at this again!
I know this is not the best time of year to add PRs, so I'm fine with postponing this if you don't feel like tackling it these upcoming weeks.
In any case, have some nice end-of-year festivities, if you celebrate any!
)

ah, and before I forget: a small part of the new test file is commented out because it hits ICE #134587, but there should be more than decent coverage anyway

unfortunately the lint needs to be gutted and rewritten.

Also while I was possibly having a mild case of get-there-itis and thus mostly tried to just make sure things were coherent, I would prefer all new code for the lint be in compiler/rustc_lint/src/types/improper_ctypes.rs.

The version cut happened so there will be less time pressure now.

I have a first version for compiler/rustc_lint/src/types/improper_ctypes.rs if you want.
it's less of a from-the-ground-up rewrite as it is scrapping the original for parts, if the analogy makes sense.

you probably have things to say about its architecture, even if the whole thing still have a bunch of TODO comments
the progress so far looks like this:

• completely separate the type-checking and reporting systems
• (part of the way there) remove some of the special cases and integrate them to the "main logic"
  • check_for_opaque_types is still a "special case" part of the checking logic
  • Cstr and Cstring are also somewhat special-cased because the advice for them depends on the type around them, if any
  • the unit type is handled in multiple places, see if this can be fixed
• (almost complete) compile, pass existing tests
  • only failed tests are for Cstring, due to different error messages
  • one unrelated test had to have a second "#[allow(improper_ctypes)]" added, but it makes more sense for it to need that anyway
• better separation of the different checks in different visit_* methods of ImproperCTypesVisitor
• better tracking of how the currently-checked type is used (static, function argument, function return's inner type, etc...)
  • raises questions about the separation of improper_ctypes and improper_ctypes_definitions versus declared/defined functions, especially when FnPtr:s are involved
• allow single argument check to emit multiple errors (for fnptr:s, structures with multiple FFI-unsafe fields, etc)
• review what is considered FFI-safe or not (once everything else is complete)

If you want to take a look in this state, should I just commit it here? (possibly put the PR in draft mode while I'm at it?)
or send you the files in a different way?

☔ The latest upstream changes (presumably #135525) made this pull request unmergeable. Please resolve the merge conflicts.

aaaand I think I have something that's "first draft" material! (should I put this pull in the "draft" state?)
There's still a bunch of TODOs (...well, they were changed to FIXMEs to be pushed here) for questions I didn't manage to answer (and a bunch of failing tests because I don't know if the error should be here), but yeah.

here's a list of some of my remaining questions and concerns:

• visit_numeric seems too x86_64-specific

• should we revisit the distinction between ImproperCTypes and ImproperCTypesDefinitions?

  • part 1: the output ("external fn" or vs "external block" vs other possibilities)
  • part 2: handling opaque types (there's a high correlation between ImproperCTypesDefinitions and places where we allow FFI-opaque types to be fully specified. do we want this correlation to be 1?)

• more on FFI-opaque types: how do we handle that in the context of the "context switch" between functions and possible FnPtr arguments? The answer that seems correct currently prevents a stage1 compiler from being built

  • should we introduce a std::ffi::FfiOpaquePtr type? (which would be a *const c_void and some phantomdata, on first approximation)

• for indirections whose values may be supplied by non-rust code: do we only allow pointers (and Optionstd::ptr::NonNull), or do we also allow Option<&T> and Option<Box<T>>?

• not sure if the new error messages are intelligible in all cases (especially if there's a type param like Self or <Self as ::std::ops::Add<Self>>::Output that gets resolved in the error message).

• it feels like the current handling of CStr/Cstring and Option-like enums uses special casing, since those are tested for in multiple places.

• if we deny references and boxes in defined functions, what of &self in methods? We don't allow *const Self, last I checked.

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

   Compiling time-core v0.1.2
   Compiling time-macros v0.2.19
   Compiling deranged v0.3.11
   Compiling rustc-main v0.0.0 (/checkout/compiler/rustc)
error: `extern` fn uses type `&RustString`, which is not FFI-safe
   |
68 |     buf: &RustString,
   |          ^^^^^^^^^^^ not FFI-safe
   |