lint: emit proper diagnostic for unsafe binders in improper_ctypes instead of ICE
Fixes rust-lang/rust#149719
This PR replaces the todo!("FIXME(unsafe_binder)") branch in the improper_ctypes lint with a proper diagnostic.
Previously, using an unsafe binder inside an extern "C" block caused an internal compiler error.
This fix now ensures that the compiler now emits a clear and stable error message explaining that types containing unsafe binders are not yet supported in FFI.
A new UI test (unsafe-binder-basic.rs) is included to ensure this behavior remains stable and prevent regressions.
r? @mati865
rustbot has assigned @mati865. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Rerolling, as I have no expertise in this area.
@rustbot reroll
I have updated the PR with the following fixes:
removed the unused _vars binding as suggested.
adjusted the UI test as per the review (removed the unnecessary // build-fail comment ).
reran the full test suite to confirm the diagnostic and stderr output are stable.
I did use an AI tool initially to draft the change, but I fully reviewed, corrected, and refined the code myself.
all changes now reflect my understanding of the lint and the unsafebinder handling.
Please let me know if anything else should be improved.
thanks for asking and happy to explain and answer your questions
i used skip_binder() because i only wanted to check the inner type that actually reaches the FFI boundary. The binder itself doesn’t matter for the FFI-safety check, since C doesn’t understand those binders. if I used the whole ty without skipping the binder, the lint wouldn’t look at the real inner type.
i understand your point about the diagnostic looking like just &().
if you prefer that the message keeps the unsafe part visible, i can update the code to print it that way.
I see your point about using the fluent error messages instead of a raw string. I will take a look at the other arms and update this one to follow the same pattern so the diagnostic style stays consistent.
I will push the fixes shortly.
The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
To only update this specific test, also pass `--test-args lint/improper-ctypes/unsafe-binder-basic.rs`
error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/lint/improper-ctypes/unsafe-binder-basic.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/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--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/aarch64-unknown-linux-gnu/test/ui/lint/improper-ctypes/unsafe-binder-basic" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error: `extern` block uses type `&()`, which is not FFI-safe
##[error] --> /checkout/tests/ui/lint/improper-ctypes/unsafe-binder-basic.rs:6:18
|
LL | fn exit_2(x: unsafe<'a> &'a ());
| ^^^^^^^^^^^^^^^^^ not FFI-safe
|
= note: types containing `unsafe` binders are not yet fully supported in FFI
note: the lint level is defined here
--> /checkout/tests/ui/lint/improper-ctypes/unsafe-binder-basic.rs:3:9
|
that makes sense.
the current output only shows &(), while the span highlights the full unsafe &a (). I agree that the diagnostic should refer to the same type for consistency.
I will update the diagnostic to report unsafe &a () instead of just &(), and then adjust the .stder file to match the updated message.
Thanks for your clear explanation and feedback. it is really helpful as i am learning my way around the compiler codebase.
Thanks for the detailed feedback I have updated the PR to address both concerns:
-
i changed the diagnostic to show the full type unsafe &a() instead of just the inner type &(). this makes it clearer that the entire unsafe binder construct is not supported in FFI.
-
switched from using a raw string to the fluent message system (added lint_improper_ctypes_unsafe_binder in messages.ftl) to keep it consistent with the other improper_ctypes handling thing.
i removed the skip_binder() call since we are not actually checking the inner type and we are just reporting that unsafe binders aren't supported in FFI contexts yet. the test output now correctly shows the full type in the error message.
let me know if there is anything else that needs adjusting. happy to learn and contribute.
hi i have squashed the commits and cleaned up the history as requested.
these are the updates included in the final commit:
-
unified the diagnostic wording and switched to the fluent message system
-
updated the error to display the full unsafe &a() type for consistency
-
removed the unnecessary skip_binder() usage
-
improved the message wording as suggested
-
updated the ui test .stder fle to match the corrected diagnostic
thank you for the guidance and feedback it really helped me understand the lint behavior much better. please let me know if anything else should be refined.
I ran ./x.py test tidy for clean format file checking because this automatically runs after running git push. So, to double check formatting i ran ./x.py test tidy and it gave errors at these two locations which you mentioned so i had to give an extra newline to pass the tidy test.
I just copied your PR and tried add them locally and tidy have nothing against this lines, can you show exact output?
compiler/rustc_lint/src/types/improper_ctypes.rs. For this file when i remove the last newline it shows me this while running tidy. so for this only i gave the the newline so that tidy test passes.
and for the messages.ftl file i removed the newline in the previous commit only.
Why the error in improper_ctypes.rs points at line 1055, when you need add new line after line 1020 (that was here before)
and for the messages.ftl file i removed the newline in the previous commit only
Yes, return it back please :) As you can see here all messages are spaced by a new line, you can't just remove it like this
Squash please
Tidy is failing because you change unrelated line, as I pointed it before
Why the error in
improper_ctypes.rspoints at line 1055, when you need add new line after line 1020
Pay attention to line number here please https://github.com/rust-lang/rust/pull/149730#discussion_r2598263663
The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
You should preserve this last trailing line in the file
I have checked the tidy issues and fixed them. Also fixed the newline and whitespaces issues. Please verify now.
Nice, thanks!
Let's wait on CI and I will merge it afterwards
Note: while I was patient and walked you through this process, other reviewers might handle things differently. In the future, similar PRs could be closed early if they require excessive back and forth for basic issues.
I'm not trying to scare you, just want you to be aware so you can better prepare for next time. If you plan to keep contributing, I'd recommend spending more time studying the surrounding code and understanding the problem fully before submitting.
Thanks for the guidance and for taking the time to walk me through the review. I understand the expectations better now, and I will make sure to study the surrounding code and prepare improvements more carefully in my future contributions. I appreciate your patience. this was very helpful for me as a new contributor.
@bors r+
:pushpin: Commit 0f4ec281558a40645b125fda43f36d7006180e54 has been approved by Kivooeo
It is now in the queue for this repository.