Repository navigation
Error on wrong targets for no_mangle and export_name - #164789
JonathanBrouwer wants to merge 2 commits into
Conversation
c344658 to
5c4119b
Compare
|
This needs a crater run |
This comment has been minimized.
This comment has been minimized.
Error on wrong targets for `no_mangle` and `export_name`,
no_mangle and export_name,no_mangle and export_name
This comment has been minimized.
This comment has been minimized.
5c4119b to
6717514
Compare
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
This comment has been minimized.
This comment has been minimized.
|
rustc_codegen_ssa has a bunch of code defending against invalid codegen attrs, being able to clean that up would be nice. (for example, #159374 adds some) |
|
☔ The latest upstream changes (presumably #164894) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
|
It looks like many errors are actually from the |
|
and from rust-analyzer: |
|
Crater analysis: small crates (crates that directly broke, or crates with <5 dependents)20 foreign static brotli12 crates cosnmic-space5 crates hdi56 crates i-slint-backend-qt27 crates qmetaobject25 crates rs-x11-hash5 crates sapp-linux24 crates |
|
The breakage in |
This is invalid as the `no_mangle` attribute has no meaning on foreign statics. This crate surfaced in a recent [crater run](rust-lang/rust#164789), where 56 crates started failing with errors like: ``` error: the `no_mangle` attribute cannot be used on foreign statics --> /opt/rustwide/cargo-home/registry/src/index.crates.io-1949cf8c6b5b557f/i-slint-backend-qt-0.3.5/qt_accessible.rs:116:9 | 116 | / cpp!(unsafe [obj as "QObject*"] { 117 | | QTimer::singleShot(0, [obj = QPointer(obj)]() { 118 | | if (!obj) 119 | | return; ... | 129 | | }); 130 | | }); | |__________^ | = help: the `no_mangle` attribute can be applied to functions and statics = note: this error originates in the macro `__cpp_closure_impl` which comes from the expansion of the macro `cpp` (in Nightly builds, run with -Z macro-backtrace for more info) ```
This PR makes applying the
#[no_mangle]and#[export_name]attributes an error, rather than a warning.It also makes applying the attributes to a provided trait function consistently an error.
This is necessary because the current behavior is very buggy, and it is not clear to me what the correct behavior should be.
History:
export_nameused to error on all targets except fields, arms, macro defs, and macro calls.no_manglehas always warned on invalid targets. It has always had buggy behaviour on provided trait functions.export_nameon provided trait functions. The behaviour is buggy.The specific changes this PR makes are:
export_namegoes from warning to error on fields, arms, macro defs, and macro callsexport_namegoes from allowed to error on provided trait functionsno_manglegoes from warning to error on all targets except the allowed targets: free functions, statics, inherent methods and trait impl methods.Fixes #164571
cc @mejrs