-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Make no_mangle on foreign items explicit instead of implicit #144678
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
base: master
Are you sure you want to change the base?
Conversation
Some changes occurred in compiler/rustc_codegen_ssa |
Can you please squash the last two commits? |
This comment has been minimized.
This comment has been minimized.
Can do |
1790424
to
5240989
Compare
5240989
to
e75b93d
Compare
e75b93d
to
9003a3b
Compare
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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+ |
…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.
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 |
Make no_mangle on foreign items explicit instead of implicit try-job: test-various
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 puttingno_mangle
in the codegen fn attrs of foreign items we can default it tono_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 #144655 which should merge first.