Skip to content

linker: Refactor library linking methods in trait Linker #120099

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

petrochenkov
Copy link
Contributor

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in trait Linker do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2024

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added 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 Jan 18, 2024
} else {
self.linker_args(&[OsString::from("--whole-archive"), path.into()]);
self.linker_arg("--whole-archive");
self.cmd.arg(path);
Copy link
Member

Choose a reason for hiding this comment

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

why is this not a linker arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For consistency with !whole_archive case above, whether -Wl, is added doesn't matter in this context.

We could probably consistently add it in all cases though, to save C compiler some work? (Looking into files and determining that they should indeed be passed through to the linker.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could probably consistently add it in all cases though, to save C compiler some work

This would be some larger work, so I've just reverted this change for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to save C compiler some work? (Looking into files and determining that they should indeed be passed through to the linker.)

Update: gcc doesn't even inspect the passed .o and .a files, it just looks at file extensions.
clang does call fstat on them, but doesn't inspect the contents either.

@WaffleLapkin
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 23, 2024

📌 Commit 42bcccc has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors 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 Jan 23, 2024
fmease added a commit to fmease/rust that referenced this pull request Jan 23, 2024
linker: Refactor library linking methods in `trait Linker`

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in `trait Linker` do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
Rollup of 12 pull requests

Successful merges:

 - rust-lang#112806 (Small code improvements in `collect_intra_doc_links.rs`)
 - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions)
 - rust-lang#119766 (Split tait and impl trait in assoc items logic)
 - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120139 (Do not normalize closure signature when building `FnOnce` shim)
 - rust-lang#120160 (Manually implement derived `NonZero` traits.)
 - rust-lang#120171 (Fix assume and assert in jump threading)
 - rust-lang#120183 (Add `#[coverage(off)]` to closures introduced by `#[test]` and `#[bench]`)
 - rust-lang#120195 (add several resolution test cases)
 - rust-lang#120259 (Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved)
 - rust-lang#120261 (Provide structured suggestion to use trait objects in some cases of `if` arm type divergence)

r? `@ghost`
`@rustbot` modify labels: rollup
@fmease
Copy link
Member

fmease commented Jan 23, 2024

Might this have caused #120279 (comment)?

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jan 23, 2024

Hmm, yes, quite likely.
Looks like .lib is used instead of .dll.lib somewhere.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 23, 2024
@petrochenkov
Copy link
Contributor Author

Fixed up the extension.
@bors r=WaffleLapkin

@bors
Copy link
Collaborator

bors commented Jan 23, 2024

📌 Commit 03f23c1 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 23, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 23, 2024
fmease added a commit to fmease/rust that referenced this pull request Jan 24, 2024
linker: Refactor library linking methods in `trait Linker`

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in `trait Linker` do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#119305 (Add `AsyncFn` family of traits)
 - rust-lang#119389 (Provide more context on recursive `impl` evaluation overflow)
 - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120201 (Bump some deps with syn 1.0 dependencies)
 - rust-lang#120230 (Assert that a single scope is passed to `for_scope`)
 - rust-lang#120278 (Remove --fatal-warnings on wasm targets)
 - rust-lang#120292 (coverage: Dismantle `Instrumentor` and flatten span refinement)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2024
linker: Refactor library linking methods in `trait Linker`

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in `trait Linker` do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#118208 (Rewrite the BTreeMap cursor API using gaps)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120165 (Switch `NonZero` alias direction.)
 - rust-lang#120288 (Bump `askama` version)
 - rust-lang#120306 (Clean up after clone3 removal from pidfd code (docs and tests))
 - rust-lang#120316 (Don't call `walk_` functions directly if there is an equivalent `visit_` method)
 - rust-lang#120330 (Remove coroutine info when building coroutine drop body)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#118208 (Rewrite the BTreeMap cursor API using gaps)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120288 (Bump `askama` version)
 - rust-lang#120306 (Clean up after clone3 removal from pidfd code (docs and tests))
 - rust-lang#120316 (Don't call `walk_` functions directly if there is an equivalent `visit_` method)
 - rust-lang#120330 (Remove coroutine info when building coroutine drop body)
 - rust-lang#120332 (Remove unused struct)
 - rust-lang#120338 (Fix links to [strict|exposed] provenance sections of `[std|core]::ptr`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 87448be into rust-lang:master Jan 26, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2024
Rollup merge of rust-lang#120099 - petrochenkov:linkapi, r=WaffleLapkin

linker: Refactor library linking methods in `trait Linker`

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in `trait Linker` do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.
@petrochenkov petrochenkov deleted the linkapi branch February 22, 2025 18:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants