Lint on combining #[no_mangle] and #[export_name] by sassman · Pull Request #131558 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

sassman

This is my very first contribution to the compiler, even though I read the chapter about lints I'm not very certain that this new lint is done right as a builtin lint PR is right. I appreciate any guidance on how to improve the code.

The mixed_export_name_and_no_mangle lint detects usage of both #[export_name] and #[no_mangle] on the same item which results on #[no_mangle] being ignored.

warn-by-default

Example

#[no_mangle] // ignored #[export_name = "foo"] // takes precedences pub fn bar() {}

Explanation

The compiler will not respect the #[no_mangle] attribute when generating the symbol name for the function, as the #[export_name] attribute takes precedence. This can lead to confusion and is unnecessary.

Fixes #47446

@rustbot

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot rustbot added 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

Oct 11, 2024

@rust-log-analyzer

This comment has been minimized.

@workingjubilee workingjubilee changed the titlefix: rust-lang/rust#47446 Lint on combining #[no_mangle] and #[export_name]

Oct 11, 2024

@workingjubilee

@Urgau Do you know why all these lints are prefixed by Builtin? Aren't all the lints in the compiler built-in by definition?

@Urgau

Do you know why all these lints are prefixed by Builtin? Aren't all the lints in the compiler built-in by definition?

It's probably a prefix for the lints defined inside compiler/rustc_lint/src/builtin.rs.

Lints outside of that file don't seems to have it at least.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee

Ah, I see.

Mm, great, in tests/ui/asm/naked-functions.rs we find this gem:

#[export_name = "exported_function_name"] #[link_section = ".custom_section"] #[no_mangle] #[naked] pub unsafe extern "C" fn compatible_ffi_attributes_1() { naked_asm!("", options(raw)); }

@sassman Thank you for PRing this lint! It looks like you will have to fix a few cases of it in our test suite as well, and --bless. Feel free to apply either, as you prefer, to relevant cases like that "naked" function.

workingjubilee

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@sassman sassman marked this pull request as ready for review

October 14, 2024 09:08

@sassman

Now the tests run all green and I have cleaned up the other test that used both no_mangle and export_name and simply removed the no_mangle in tests/ui/asm/naked-functions.rs.

Please let me know if there is anything that I can do further.

@Urgau

@Urgau Urgau added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Oct 14, 2024

@sassman

Thanks @Urgau for the guidance, I will refactor the code accordingly.
The provided example seems pretty helpful to figure things out.

@rust-log-analyzer

This comment has been minimized.

@sassman

Urgau

Urgau

@Urgau

@Urgau Urgau added the S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

label

Dec 9, 2024

@rust-log-analyzer

This comment has been minimized.

@sassman @Urgau

commit suggestion of not always pretty printing

Co-authored-by: Urgau 3616612+Urgau@users.noreply.github.com

@sassman

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Dec 9, 2024

Urgau

@Urgau

@bors

📌 Commit 7951d19 has been approved by Urgau

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Dec 9, 2024

fmease added a commit to fmease/rust that referenced this pull request

Dec 9, 2024

@fmease

…together-with-export-name, r=Urgau

Lint on combining #[no_mangle] and #[export_name]

This is my very first contribution to the compiler, even though I read the chapter about lints I'm not very certain that this new lint is done right as a builtin lint PR is right. I appreciate any guidance on how to improve the code.

Old proposed new lint

The mixed_export_name_and_no_mangle lint detects usage of both #[export_name] and #[no_mangle] on the same item which results on #[no_mangle] being ignored.

warn-by-default

Example

#[no_mangle] // ignored
#[export_name = "foo"] // takes precedences
pub fn bar() {}

Explanation

The compiler will not respect the #[no_mangle] attribute when generating the symbol name for the function, as the #[export_name] attribute takes precedence. This can lead to confusion and is unnecessary.

fmease added a commit to fmease/rust that referenced this pull request

Dec 9, 2024

@fmease

…together-with-export-name, r=Urgau

Lint on combining #[no_mangle] and #[export_name]

This is my very first contribution to the compiler, even though I read the chapter about lints I'm not very certain that this new lint is done right as a builtin lint PR is right. I appreciate any guidance on how to improve the code.

Old proposed new lint

The mixed_export_name_and_no_mangle lint detects usage of both #[export_name] and #[no_mangle] on the same item which results on #[no_mangle] being ignored.

warn-by-default

Example

#[no_mangle] // ignored
#[export_name = "foo"] // takes precedences
pub fn bar() {}

Explanation

The compiler will not respect the #[no_mangle] attribute when generating the symbol name for the function, as the #[export_name] attribute takes precedence. This can lead to confusion and is unnecessary.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Dec 10, 2024

@bors

Rollup of 10 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Dec 10, 2024

@bors

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Dec 10, 2024

@rust-timer

Rollup merge of rust-lang#131558 - sassman:feat/warnin-for-no-mangle-together-with-export-name, r=Urgau

Lint on combining #[no_mangle] and #[export_name]

This is my very first contribution to the compiler, even though I read the chapter about lints I'm not very certain that this new lint is done right as a builtin lint PR is right. I appreciate any guidance on how to improve the code.

Old proposed new lint

The mixed_export_name_and_no_mangle lint detects usage of both #[export_name] and #[no_mangle] on the same item which results on #[no_mangle] being ignored.

warn-by-default

Example

#[no_mangle] // ignored
#[export_name = "foo"] // takes precedences
pub fn bar() {}

Explanation

The compiler will not respect the #[no_mangle] attribute when generating the symbol name for the function, as the #[export_name] attribute takes precedence. This can lead to confusion and is unnecessary.

@sassman sassman deleted the feat/warnin-for-no-mangle-together-with-export-name branch

December 11, 2024 22:05

@Kobzol

@rust-timer

This comment has been minimized.

@rust-timer

Finished benchmarking commit (79254c6): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -3.0%, secondary -0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 2.8% [2.8%, 2.8%] 1
Improvements ✅ (primary) -3.0% [-4.7%, -1.3%] 2
Improvements ✅ (secondary) -3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) -3.0% [-4.7%, -1.3%] 2

Cycles

Results (secondary 2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 2.1% [2.0%, 2.3%] 2
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) - - 0

Binary size

Results (secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -0.0% [-0.0%, -0.0%] 1
All ❌✅ (primary) - - 0

Bootstrap: 767.346s -> 766.133s (-0.16%)
Artifact size: 330.96 MiB -> 330.98 MiB (0.00%)

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Feb 23, 2025

@he32

Pkgsrc changes relative to rust184:

Version 1.85.0 (2025-02-20)

Language

Compiler

Platform Support

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request

Feb 26, 2025

@tmeijn

This MR contains the following updates:

Package Update Change
rust minor 1.84.1 -> 1.85.0

MR created with the help of el-capitano/tools/renovate-bot.

Proposed changes to behavior should be submitted there as MRs.

Release Notes

rust-lang/rust (rust)

v1.85.0

Compare Source

==========================

Language

Compiler

Platform Support

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻ Rebasing: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this MR and you won't be reminded about this update again.

This MR has been generated by Renovate Bot.