Lower #[no_mangle] to #[symbol_name]/#[link_name] by bjorn3 · Pull Request #159374 · rust-lang/rust

bjorn3 · 2026-07-16T10:22:36Z

View all comments

This reduces the amount of special casing #[no_mangle] needs in the rest of the compiler.

t-lang: nomination comment, example

rustbot · 2026-07-16T10:22:40Z

miri is developed in its own repository. If possible, consider making this change to rust-lang/miri instead.

cc @rust-lang/miri

rustbot · 2026-07-16T10:22:42Z

r? @jackh726

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: codegen, compiler
codegen, compiler expanded to 75 candidates
Random selection from 19 candidates

bjorn3 · 2026-07-16T14:37:08Z

        } else if codegen_fn_attrs.symbol_name.is_some() {
            // * This can be overridden with the `#[link_name]` attribute
+        } else if codegen_fn_attrs.link_ordinal.is_some() {
+            // * `#[link_ordinal]` and `#[link_name]` are incompatible with each other


This is a behavior change. We no longer keep the symbol that Rust code refers to unmangled when #[link_ordinal] is used. This matches the behavior on wasm for #[wasm_import_section], but prevents overriding the symbol using a definition with an #[export_name] matching the name we have imported it as in the raw-dylib block. And it is inconsistent with non-ordinal imports through raw-dylib where we could also have used mangled names on our side.

View changes since the review

I think that makes sense, what's the process here? Nominate for t-lang and let them decide whether to accept this change?

Ideally, separate refactors from behavior changes. Doing both in the same PR makes things more complicated to manage.

It is non-trivial to keep the old behavior after this refactor. I could open a PR to change the behavior first.

I think that would be better, yeah.

And it is inconsistent with non-ordinal imports through raw-dylib where we could also have used mangled names on our side.

It is unclear to me whether you are calling the old or new behavior inconsistent here.

Ideally, separate refactors from behavior changes.

I think the current PR is small enough to get away with this, no need to split it up here.

However, you can probably simplify much more by lowering into the same attribute during attribute parsing instead. (that would be a more intrusive change where refactors and behavior changes should be split)

It is unclear to me whether you are calling the old or new behavior inconsistent here.

The old behavior is consistent with non-ordinal imports on windows. The new behavior is consistent with imports on wasm, but not with non-ordinal imports on windows.

RalfJung · 2026-08-07T16:46:42Z

@rustbot reroll

JonathanBrouwer · 2026-08-14T06:43:27Z

@rustbot reroll
Not familiar enough with link_ordinal to feel comfortable reviewing this

mejrs

My understanding here is that no_mangle is just a convenience for export_name = "<item_name>", is that correct? What is the interaction with link_name?

Although while reviewing I did notice that this is allowed:

pub trait Trait {
    #[unsafe(export_name = "method")]
    fn method(&self) {}
}

but this emits a warning:

pub trait Trait {
    #[unsafe(no_mangle)]
    fn method(&self) {}
}

This looks like a bug (none or both should be allowed here)

View changes since this review

mejrs

r=me on the implementation, IMO the new behavior changes make sense also. But I think this sort of thing is ultimately t-lang's decision, can you nominate it for them to discuss?

View changes since this review

bjorn3 · 2026-08-20T13:08:43Z

The exact change nominated for the lang team is #159374 (comment) Basically an item with #[unsafe(link_ordinal)] inside a raw-dylib block on Windows will now always import this ordinal from the target DLL rather than being overridable by a local definition of a symbol with a symbol name matching the item name of the item with #[unsafe(link_ordinal)] (the item name has no relation to the name of the imported symbol, as for ordinal exports there is no symbol name in the first place).

The only uses of #[link_ordinal] I found are: https://github.com/search?q=%2F%5B%5C%5B%5C%28%5Dlink_ordinal%5C%28%5Cd%2B%5C%29%2F+language%3Arust+-is%3Afork+-path%3A%2Ftests%5C%2F%2F+-path%3A%2Fsrc%5C%2Ftest%5C%2F%2F&type=code

Both of these refer to undocumented functions in uxtheme.dll. I don't think the item names these used are actually their official names as opposed to just a random name picked by the respective crate author that they might change at any time and would thus not be safe to override by defining a function with the same name even if it was possible.

RalfJung · 2026-08-20T13:37:58Z

Basically an item with #[unsafe(link_ordinal)] inside a raw-dylib block on Windows will now always import this ordinal from the target DLL rather than being overridable by a local definition of a symbol with a symbol name matching the item name of the item with #[unsafe(link_ordinal)] (the item name has no relation to the name of the imported symbol, as for ordinal exports there is no symbol name in the first place).

Can you give an example of a piece of code that changed its meaning, and describe the old and new meaning?

bjorn3 · 2026-08-20T13:58:14Z

mod ffi {
    #[link(name = "foo", kind = "raw-dylib")]
    unsafe extern "C" {
        #[unsafe(link_ordinal(7))] // Perhaps internally called MyFancyFunction when foo.dll was compiled
        pub safe fn barf();
    }
}

#[unsafe(no_mangle)]
extern "C" fn barf() {
    println!("Override called");
}

fn main() {
    ffi::barf();
}

would previously cause the override to be called, but will now call ordinal export 7 of foo.dll like would have been the case without the override.

RalfJung · 2026-08-20T14:01:05Z

Wow yeah the old behavior seems really strange there. I expected barf to be just a local name for the import that's otherwise irrelevant, given the attribute.

And it is inconsistent with non-ordinal imports through raw-dylib where we could also have used mangled names on our side.

This sounds like we are still doing the strange behavior in some other related case?

mejrs · 2026-11-03T17:36:59Z

Oh, this also needs a reference PR, can you open one?

bjorn3 · 2026-11-04T15:07:31Z

Opened rust-lang/reference#2406

mejrs

@bors r+ rollup

View changes since this review

rust-bors · 2026-11-04T15:27:56Z

📌 Commit 460bb55 has been approved by mejrs

It is now in the queue for this repository.

Lower #[no_mangle] to #[symbol_name]/#[link_name] This reduces the amount of special casing #[no_mangle] needs in the rest of the compiler. t-lang: [nomination comment](rust-lang#159374 (comment)), [example](rust-lang#159374 (comment))

Rollup of 13 pull requests Successful merges: - #163477 (Fancy paramenv accessors that communicate the access pattern better.) - #160593 (turn ABI-controlling target features into a target modifier) - #164896 (offload: pass the linked device image to the host pass) - #165468 (Remove return type from `BuilderMethods::store`) - #159374 (Lower #[no_mangle] to #[symbol_name]/#[link_name]) - #164564 (Re-enable outline-atomics by default on more AArch64 platforms) - #164623 (add next-solver regression tests for two fixed ices) - #164678 (add next-solver regression test for recursive opaque ICE under validate-mir) - #165429 (Add tests for `-Zcrate-attr` edition and position) - #165430 ([tiny] Show context for found assoc item in E0107) - #165456 (std: Fix build for the L4Re targets) - #165463 (Add BTreeMap::allocator and BTreeSet::allocator) - #165466 (Don't "exit" using `panic` on teeos)

JonathanBrouwer · 2026-11-05T08:13:56Z

@bors try jobs=test-x86_64-mingw-1

Lower #[no_mangle] to #[symbol_name]/#[link_name] try-job: test-x86_64-mingw-1

rust-bors · 2026-11-05T11:02:10Z

💔 Test for 13435e5 failed: CI. Failed job:

try - test-x86_64-mingw-1 (web logs, enhanced plaintext logs)

JonathanBrouwer · 2026-11-05T11:07:42Z

@bors r-

rust-bors · 2026-11-05T11:07:47Z

This pull request was unapproved.

View changes since this unapproval

This ensures the name it is referenced with from object files matches the name that it is imported as in an import library.

bjorn3 · 2026-11-05T12:37:19Z

@bors try jobs=test-x86_64-mingw-1

Lower #[no_mangle] to #[symbol_name]/#[link_name] try-job: test-x86_64-mingw-1

rust-bors · 2026-11-05T15:41:09Z

☀️ Try build successful (CI)
Build commit: 1906f55 (1906f555daa1f1430ffbcb378cf7b468c8418bc7)
Base parent: 70424d8 (70424d844bbe2c6dbc8f38dda80affd754a7cfe4)

bjorn3 · 2026-11-05T15:47:41Z

@rustbot ready

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 Jul 16, 2026

rustbot assigned jackh726 Jul 16, 2026

This comment has been minimized.

Sign in to view

bjorn3 commented Jul 16, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

bjorn3 force-pushed the desugar_no_mangle branch from 85e13aa to d86eb41 Compare July 20, 2026 14:30

bjorn3 mentioned this pull request Jul 24, 2026

Rename symbols rather than add symbol aliases for EIIs when possible #159843

Draft

bjorn3 force-pushed the desugar_no_mangle branch from d86eb41 to 22ac858 Compare August 4, 2026 12:50

This comment has been minimized.

Sign in to view

rustbot assigned JonathanBrouwer and unassigned jackh726 Aug 7, 2026

rustbot assigned mejrs and unassigned JonathanBrouwer Aug 14, 2026

mejrs reviewed Aug 16, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_ssa/src/codegen_attrs.rs Resolved

mejrs approved these changes Aug 20, 2026

View reviewed changes

bjorn3 added the I-lang-nominated Nominated for discussion during a lang team meeting. label Aug 20, 2026

This comment was marked as resolved.

Sign in to view

bjorn3 mentioned this pull request Nov 4, 2026

Update #[link_ordinal] behavior rust-lang/reference#2406

Open

mejrs removed the needs-reference-pr This language change needs an approved Reference PR to proceed. label Nov 4, 2026

mejrs approved these changes Nov 4, 2026

View reviewed changes

rust-bors Bot 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 Nov 4, 2026

jhpratt mentioned this pull request Nov 5, 2026

Rollup of 12 pull requests #165480

Closed

jhpratt mentioned this pull request Nov 5, 2026

Rollup of 13 pull requests #165488

Closed

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Nov 5, 2026

Auto merge of #159374 - bjorn3:desugar_no_mangle, r=<try>

13435e5

Lower #[no_mangle] to #[symbol_name]/#[link_name] try-job: test-x86_64-mingw-1

rust-bors Bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 5, 2026

rust-bors Bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 5, 2026

This comment has been minimized.

Sign in to view

Use symbol name in build_dll_import

b4b982c

This ensures the name it is referenced with from object files matches the name that it is imported as in an import library.

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Nov 5, 2026

Auto merge of #159374 - bjorn3:desugar_no_mangle, r=<try>

1906f55

Lower #[no_mangle] to #[symbol_name]/#[link_name] try-job: test-x86_64-mingw-1

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 Nov 5, 2026

Conversation

bjorn3 commented Jul 16, 2026 • edited by RalfJung Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 16, 2026

Uh oh!

rustbot commented Jul 16, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

bjorn3 Jul 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

bjorn3 Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

bjorn3 Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

RalfJung commented Aug 7, 2026

Uh oh!

JonathanBrouwer commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjorn3 commented Aug 20, 2026

Uh oh!

RalfJung commented Aug 20, 2026

Uh oh!

bjorn3 commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Aug 20, 2026

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

mejrs commented Nov 3, 2026

Uh oh!

bjorn3 commented Nov 4, 2026

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Nov 4, 2026

Uh oh!

JonathanBrouwer commented Nov 5, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Nov 5, 2026

Uh oh!

JonathanBrouwer commented Nov 5, 2026

Uh oh!

rust-bors Bot commented Nov 5, 2026

Uh oh!

bjorn3 commented Jul 16, 2026 •

edited by RalfJung

Loading

bjorn3 Jul 16, 2026 •

edited by rustbot

Loading

RalfJung Aug 16, 2026 •

edited

Loading

JonathanBrouwer commented Aug 14, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading

bjorn3 commented Aug 20, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading