Don't mark reachable extern fns as internal by alexcrichton · Pull Request #10539 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits1 Checks0 Files changed

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 }})

If a function is marked as external, then it's likely desired for use with some
native library, so we're not really accomplishing a whole lot by internalizing
all of these symbols.

This is the implementation that I wanted to see of #9945

I again don't see why this should be tied to ABI. Why not just make fully public (reachable) paths in a binary external like clang does? An LTO pass can internalize everything but main without hitting the problems we do.

The rationale for adding the building_library check was due to reachable not working very well. Assuming it works well, the only difference between a executable and a library should be that #[inline] doesn't make functions get considered reachable in an executable.

huonw

fn main() {
unsafe {
let a = DynamicLibrary::open(None).unwrap();
assert!(a.symbol::("fun1").is_ok());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could call these external1, ..., internal1, ..., rather than relying on this assertion chain to describe the intended behaviour.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Relying on just this assertion chain.)

bors added a commit that referenced this pull request

Nov 18, 2013

If a function is marked as external, then it's likely desired for use with some native library, so we're not really accomplishing a whole lot by internalizing all of these symbols.

bors added a commit that referenced this pull request

Nov 18, 2013

If a function is marked as external, then it's likely desired for use with some native library, so we're not really accomplishing a whole lot by internalizing all of these symbols.

flip1995 pushed a commit to flip1995/rust that referenced this pull request

Mar 24, 2023

Rustup

r? @ghost changelog: none

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

Jan 13, 2025

…, r=

tests: Port extern-fn-reachable to rmake.rs

Part of rust-lang#121876.

Summary

This PR ports tests/run-make/extern-fn-reachable to use rmake.rs. Notable changes:

We now use the object crate and look at the exported symbols specifically.
This test's coverage regressed against windows-msvc back in replace dynamic library module with libloading rust-lang#90716, but since we use object now, we're able to claw the test coverage back.
The checks are now stricter:
1. It no longer looks for substring symbol matches in nm textual outputs, it inspects the symbol names precisely.
2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets.

History

Test was initially introduced as a run-pass[^run-pass] test as part of Don't mark reachable extern fns as internal rust-lang#10539.
Test re-introduced as a run-make test in rust-lang#13741.
Later, the test coverage regressed in rust-lang#90716.

[^run-pass]: no longer a thing nowadays

Supersedes rust-lang#128314. Co-authored with @lolbinarycat.

r? @ghost

try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

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

Jan 14, 2025

…, r=

tests: Port extern-fn-reachable to rmake.rs

Part of rust-lang#121876.

Summary

This PR ports tests/run-make/extern-fn-reachable to use rmake.rs. Notable changes:

We now use the object crate and look at the exported symbols specifically.
This test's coverage regressed against windows-msvc back in replace dynamic library module with libloading rust-lang#90716, but since we use object now, we're able to claw the test coverage back.
The checks are now stricter:
1. It no longer looks for substring symbol matches in nm textual outputs, it inspects the symbol names precisely.
2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets.
Added another case of #[no_mangle] fn fun6() {} (note the lack of pub) to check that Rust nameres visibility is orthogonal to symbol visiblity in dylib.

History

Test was initially introduced as a run-pass[^run-pass] test as part of Don't mark reachable extern fns as internal rust-lang#10539.
Test re-introduced as a run-make test in rust-lang#13741.
Later, the test coverage regressed in rust-lang#90716.

[^run-pass]: no longer a thing nowadays

Supersedes rust-lang#128314. Co-authored with @lolbinarycat.

r? @ghost

try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

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

Jan 16, 2025

…, r=lqd

tests: Port extern-fn-reachable to rmake.rs

Part of rust-lang#121876.

Summary

This PR ports tests/run-make/extern-fn-reachable to use rmake.rs. Notable changes:

We now use the object crate and look at the exported symbols specifically.
This test's coverage regressed against windows-msvc back in replace dynamic library module with libloading rust-lang#90716, but since we use object now, we're able to claw the test coverage back.
The checks are now stricter:
1. It no longer looks for substring symbol matches in nm textual outputs, it inspects the symbol names precisely.
2. We now also explicitly check for the presence of leading underscore in exported symbol names on apple vs non-apple targets.
Added another case of #[no_mangle] fn fun6() {} (note the lack of pub) to check that Rust nameres visibility is orthogonal to symbol visibility in dylib.

History

Test was initially introduced as a run-pass[^run-pass] test as part of Don't mark reachable extern fns as internal rust-lang#10539.
Test re-introduced as a run-make test in rust-lang#13741.
Later, the test coverage regressed in rust-lang#90716.

[^run-pass]: no longer a thing nowadays

Supersedes rust-lang#128314. Co-authored with @lolbinarycat.

try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various