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

Remove target_override #122810

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Remove target_override #122810

merged 2 commits into from
Mar 21, 2024

Conversation

nnethercote
Copy link
Contributor

Because the "target can override the backend" and "backend can override the target" situation is a mess. Details in the individual commits.

r? @WaffleLapkin

Backend and target selection is a mess: the target can override the
backend (via `Target::default_codegen_backend`), *and* the backend can
override the target (via `CodegenBackend::target_override`).

The code that handles this is ugly. It calls `build_target_config`
twice, once before getting the backend and once again afterward. It also
must check that both overrides aren't triggering at the same time.

This commit removes the latter override. It's used in rust-gpu but
@eddyb said via Zulip that removing it would be ok. This simplifies the
code greatly, and will allow some nice follow-up refactorings.
Because the underlying type is called `Target`. (There is also a
separate type called `TargetCfg`.)
@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 Mar 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2024

These commits modify compiler targets.
(See the Target Tier Policy.)

Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

Thanks!

@WaffleLapkin
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 21, 2024

📌 Commit 5744be2 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 Mar 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122402 (Make `#[diagnostic::on_unimplemented]` format string parsing more robust)
 - rust-lang#122644 (pattern analysis: add a custom test harness)
 - rust-lang#122733 (Strip placeholders from hidden types before remapping generic parameter)
 - rust-lang#122752 (Interpolated cleanups)
 - rust-lang#122771 (add some comments to hir::ModuleItems)
 - rust-lang#122793 (Implement macro-based deref!() syntax for deref patterns)
 - rust-lang#122810 (Remove `target_override`)
 - rust-lang#122827 (Remove unnecessary braces from `bug`/`span_bug`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 55a9165 into rust-lang:master Mar 21, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2024
Rollup merge of rust-lang#122810 - nnethercote:rm-target_override, r=WaffleLapkin

Remove `target_override`

Because the "target can override the backend" and "backend can override the target" situation is a mess. Details in the individual commits.

r? `@WaffleLapkin`
@nnethercote nnethercote deleted the rm-target_override branch March 21, 2024 22:58
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.

4 participants