-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Don't mark reachable extern fns as internal #10539
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
Conversation
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 The rationale for adding the |
fn main() { | ||
unsafe { | ||
let a = DynamicLibrary::open(None).unwrap(); | ||
assert!(a.symbol::<int>("fun1").is_ok()); |
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.
You could call these external1
, ..., internal1
, ..., rather than relying on this assertion chain to describe the intended behaviour.
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.
(Relying on just this assertion chain.)
(also, statics) |
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.
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.
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.
Rustup r? `@ghost` changelog: none
…, r=<try> 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](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](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
…, r=<try> 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](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](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
…, 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](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](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
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.