Make no_mangle on foreign items explicit instead of implicit #144678
+72
−54
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
rustbot
added
A-attributes
|
Some changes occurred in compiler/rustc_codegen_ssa |
bjorn3
reviewed
Jul 30, 2025
|
Can you please squash the last two commits? |
This comment has been minimized.
This comment has been minimized.
|
Can do |
jdonszelmann
force-pushed
the
no-mangle-extern
branch
2 times, most recently
from
1790424 to
5240989
Compare
bjorn3
reviewed
Jul 31, 2025
jdonszelmann
force-pushed
the
no-mangle-extern
branch
from
5240989 to
e75b93d
Compare
bjorn3
reviewed
Jul 31, 2025
jdonszelmann
force-pushed
the
no-mangle-extern
branch
from
e75b93d to
9003a3b
Compare
bjorn3
reviewed
Jul 31, 2025
| @@ -192,6 +192,10 @@ impl CodegenFnAttrs { | |||
| /// * `#[export_name(...)]` is present | |||
| /// * `#[linkage]` is present | |||
| /// | |||
| /// Note that this returns true for foreign items. | |||
| /// However, in some places that care about `contains_extern_indicator`, foreign items | |||
| /// (in an `extern` block) should explicitly be ignored. | |||
There was a problem hiding this comment.
Is it possible to make this not return true for foreign items? Foreign items don't need to be exported and the point of this method is to check if an item needs to be exported.
There was a problem hiding this comment.
it's annoying since we don't have a did or TyCtxt here. We could, but that'd involve adding another flag in the codegen fn attributes. I decided against that but it's perfectly possible.
|
@bors r+ |
bors
added
S-waiting-on-bors
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Aug 3, 2025
…jorn3 Make no_mangle on foreign items explicit instead of implicit for a followup PR I'm working on I need some foreign items to mangle. I could add a new attribute: `no_no_mangle` or something silly like that but by explicitly putting `no_mangle` in the codegen fn attrs of foreign items we can default it to `no_mangle` and then easily remove it when we don't want it. I guess you'd know about this r? `@bjorn3.` Shouldn't be too hard to review :) Builds on rust-lang#144655 which should merge first.
bors
added a commit
that referenced
this pull request
Aug 3, 2025
Rollup of 13 pull requests Successful merges: - #143857 (Port #[macro_export] to the new attribute parsing infrastructure) - #144070 (Implement `hash_map` macro ) - #144322 (Add lint against dangling pointers from local variables) - #144667 (`AlignmentEnum` should just be `repr(usize)` now) - #144678 (Make no_mangle on foreign items explicit instead of implicit) - #144790 (Multiple bounds checking elision failures) - #144794 (Port `#[coroutine]` to the new attribute system) - #144805 (compiletest: Preliminary cleanup of `ProcRes` printing/unwinding) - #144808 (`Interner` arg to `EarlyBinder` does not affect auto traits) - #144816 (Update E0562 to account for the new impl trait positions) - #144822 (Return a struct with named fields from `hash_owner_nodes`) - #144824 (Updated test links in compiler) - #144829 (Use full flag name in strip command for Darwin) r? `@ghost` `@rustbot` modify labels: rollup
|
Trying to diagnose rollup failure #144846 (comment) @bors try jobs=test-various |
rust-bors bot
added a commit
that referenced
this pull request
Aug 3, 2025
Make no_mangle on foreign items explicit instead of implicit try-job: test-various
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed (CI). Failed jobs:
|
|
@bors r- |
bors
added
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
for a followup PR I'm working on I need some foreign items to mangle. I could add a new attribute:
no_no_mangleor something silly like that but by explicitly puttingno_manglein the codegen fn attrs of foreign items we can default it tono_mangleand then easily remove it when we don't want it.I guess you'd know about this r? @bjorn3. Shouldn't be too hard to review :)
Builds on #144655 which should merge first.