Add a special case for CStr/CString in the improper_ctypes lint

[Flying-Toast]

Instead of saying to "consider adding a #[repr(C)] or #[repr(transparent)] attribute to this struct", we now tell users to "Use *const ffi::c_char instead, and pass the value from CStr::as_ptr()" when the type involved is a CStr or a CString.

Inspired by a conversation on the #beginners Discord channel.

[rustbot]

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

[asquared31415]

I wonder if there could be better suggestions that use binding.as_ptr() instead of CStr::as_ptr? I don't think it really matters though.

[Flying-Toast]

I wonder if there could be better suggestions that use binding.as_ptr() instead of CStr::as_ptr? I don't think it really matters though.

binding.as_ptr() would require the error to be emitted at a call site, rather than at the definition/declaration though right? Or are you talking about including the literal string "binding" in the error?

[Flying-Toast]

r? @cjgillot

[rustbot]

Could not assign reviewer from: cjgillot.
User(s) cjgillot are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

[bors]

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

[rustbot]

Some changes occurred in src/tools/cargo

cc @ehuss

[Flying-Toast]

Oops, didn't mean to touch all the submodules

[cjgillot]

@bors r+

[bors]

📌 Commit 4ea52f1 has been approved by cjgillot

It is now in the queue for this repository.

[bors]

⌛ Testing commit 4ea52f1 with merge 0f5b39f...

[rust-log-analyzer]

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

Click to see the possible cause of the failure (guessed by this bot)

---- [ui] tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.rs stdout ----
diff of stderr:

- warning: `extern` fn uses type `[i8 or u8 (arch dependant)]`, which is not FFI-safe
+ warning: `extern` fn uses type `CStr`, which is not FFI-safe
3    |
4 LL | type Foo = extern "C" fn(::std::ffi::CStr);

5    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
5    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
6    |
-    = help: consider using a raw pointer instead
-    = note: slices have no C equivalent
+    = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+    = note: `CStr`/`CString` do not have a guaranteed layout
9    = note: `#[warn(improper_ctypes_definitions)]` on by default
10 
- warning: `extern` block uses type `[i8 or u8 (arch dependant)]`, which is not FFI-safe
+ warning: `extern` block uses type `CStr`, which is not FFI-safe
13    |
13    |
14 LL |     fn meh(blah: Foo);
15    |                  ^^^ not FFI-safe
16    |
-    = help: consider using a raw pointer instead
-    = help: consider using a raw pointer instead
-    = note: slices have no C equivalent
+    = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+    = note: `CStr`/`CString` do not have a guaranteed layout
19    = note: `#[warn(improper_ctypes)]` on by default
21 warning: 2 warnings emitted


The actual stderr differed from the expected stderr.
The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-C-non-FFI-safe-arg-ice-52334/extern-C-non-FFI-safe-arg-ice-52334.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args extern/extern-C-non-FFI-safe-arg-ice-52334.rs`

error: 1 errors occurred comparing output.
status: exit status: 0
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/checkout/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-C-non-FFI-safe-arg-ice-52334" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/extern/extern-C-non-FFI-safe-arg-ice-52334/auxiliary"
--- stderr -------------------------------
--- stderr -------------------------------
warning: `extern` fn uses type `CStr`, which is not FFI-safe
   |
LL | type Foo = extern "C" fn(::std::ffi::CStr);
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
   |
   |
   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
   = note: `CStr`/`CString` do not have a guaranteed layout
   = note: `#[warn(improper_ctypes_definitions)]` on by default

warning: `extern` block uses type `CStr`, which is not FFI-safe
   |
   |
LL |     fn meh(blah: Foo);
   |                  ^^^ not FFI-safe
   |
   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
   = note: `CStr`/`CString` do not have a guaranteed layout
   = note: `#[warn(improper_ctypes)]` on by default
warning: 2 warnings emitted
------------------------------------------

[bors]

💔 Test failed - checks-actions

[fmease]

(sync)
@bors r-

[oskgo]

@Flying-Toast Can you update/fix the failing tests?

[Dylan-DPC]

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks