Skip to content

Issue an error or warning when using no_mangle on language items #140203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexandruradovici
Copy link

@alexandruradovici alexandruradovici commented Apr 23, 2025

This pull requests adds the code to issue an error or a warning when using no_mangle on language items. This should detail why the undefined symbol error is issued for the code described in #139923.

The pull request adds two ui tests, one testing the error and the other one the warning.

I would love some feedback here, as I am not sure that the error and warning are issues using the right API.

@rustbot
Copy link
Collaborator

rustbot commented Apr 23, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 23, 2025
@alexandruradovici
Copy link
Author

r? @bjorn3

@alexandruradovici alexandruradovici force-pushed the error_for_no_mangle_weak_language_items branch from f599390 to caa2b04 Compare April 23, 2025 14:58
@rust-log-analyzer

This comment has been minimized.

@alexandruradovici
Copy link
Author

alexandruradovici commented Apr 23, 2025

This fails due to a test that defines #[panic_handler] in a wrong place that also defines #[no_mangle].

#[panic_handler] //~ ERROR `panic_impl` lang item must be applied to a function
#[no_mangle]
static X: u32 = 42;

This somehow seems to be an error priority issue, the error that this pull request issues is correct.

I am not sure exactly how to avoid this, as I don't think checking that the target is a function is correct, as some language items are not functions.

@Noratrieb
Copy link
Member

This fails due to a test that defines #[panic_handler] in a wrong place that also defines #[no_mangle].

#[panic_handler] //~ ERROR `panic_impl` lang item must be applied to a function
#[no_mangle]
static X: u32 = 42;

This somehow seems to be an error priority issue, the error that this pull request issues is correct.

I am not sure exactly how to avoid this, as I don't think checking that the target is a function is correct, as some language items are not functions.

you can probably just fix the test if you look at the history of the test and ensure that what it's trying to test is still tested

@alexandruradovici
Copy link
Author

alexandruradovici commented Apr 25, 2025

Thank you @Noratrieb

I deleted the no_mangle attribute from the ui/panic-handler/panic-handler-wrong-location. I am not sure why that was placed there, but as it now is an error, I think this should be correct. I might be wrong though, so any feedback is welcome.

My understanding is that the test should verify that the panic_handler attribute is used on a function. The no_mangle attribute should impact it in any way. The test still issues the same error, panic_impl lang item must be applied to a function, when the no_mangle is deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants