Skip to content
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

rustc_target: Update some old naming around self contained linking #100126

Merged
merged 1 commit into from
Aug 14, 2022

Conversation

petrochenkov
Copy link
Contributor

The "fallback" naming pre-dates introduction of -Clink-self-contained.
Noticed when reviewing #99500.

This PR doesn't break any json target spec, but supporting per-linker-flavor startup objects needed by #99500 will break them, so maybe next time I'll remove the compatibility names.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 3, 2022
@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 3, 2022
"mingw" => CrtObjectsFallback::Mingw,
"wasm" => CrtObjectsFallback::Wasm,
"false" => LinkSelfContainedDefault::False,
"true" | "wasm" => LinkSelfContainedDefault::True,
Copy link
Member

Choose a reason for hiding this comment

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

Is the wasm case here for back-compat for any out-of-tree target specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I doubt that it's used anywhere, but I just kept full backward compatibility for all options for now.

@luqmana
Copy link
Member

luqmana commented Aug 5, 2022

+1 This naming makes the option a lot less confusing. LGTM

@lcnr
Copy link
Contributor

lcnr commented Aug 6, 2022

r? compiler

@rust-highfive rust-highfive assigned fee1-dead and unassigned lcnr Aug 6, 2022
/// Same as `(pre|post)_link_objects`, but when self-contained linking mode is enabled.
pub pre_link_objects_self_contained: CrtObjects,
pub post_link_objects_self_contained: CrtObjects,
pub link_self_contained_default: LinkSelfContainedDefault,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
pub link_self_contained_default: LinkSelfContainedDefault,
pub link_self_contained: LinkSelfContainedDefault,

Large number of target options target.foo provide defaults for something that can also be user-specified -Cfoo=override, and almost none of them uses the _default suffix.

@fee1-dead
Copy link
Member

r? compiler

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 9, 2022

📌 Commit e74299b has been approved by davidtwco

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 Aug 9, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 10, 2022
rustc_target: Update some old naming around self contained linking

The "fallback" naming pre-dates introduction of `-Clink-self-contained`.
Noticed when reviewing rust-lang#99500.

This PR doesn't break any json target spec, but supporting per-linker-flavor startup objects needed by rust-lang#99500 will break them, so maybe next time I'll remove the compatibility names.
@bors

This comment was marked as resolved.

@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 Aug 12, 2022
The "fallback" naming pre-dates introduction of `-Clink-self-contained`
@petrochenkov
Copy link
Contributor Author

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented Aug 12, 2022

📌 Commit 8fa707a has been approved by davidtwco

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 12, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 13, 2022
rustc_target: Update some old naming around self contained linking

The "fallback" naming pre-dates introduction of `-Clink-self-contained`.
Noticed when reviewing rust-lang#99500.

This PR doesn't break any json target spec, but supporting per-linker-flavor startup objects needed by rust-lang#99500 will break them, so maybe next time I'll remove the compatibility names.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#99582 (Delay a span bug if we see ty/const generic params during writeback)
 - rust-lang#99861 (orphan check: rationalize our handling of constants)
 - rust-lang#100026 (Add `Iterator::array_chunks` (take N+1))
 - rust-lang#100115 (Suggest removing `let` if `const let` or `let const` is used)
 - rust-lang#100126 (rustc_target: Update some old naming around self contained linking)
 - rust-lang#100487 (`assert_{inhabited,zero_valid,uninit_valid}` intrinsics are safe)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 38bc937 into rust-lang:master Aug 14, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

8 participants