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

fix: remove the check of lld not supporting @response-file #138432

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

weihanglo
Copy link
Member

In LLVM v9, lld has supported @response-file.
LLVM v9 was released on 2019-09-19.
The check was added back to 2018-03-14 (1.26.0) via 04442af.
It has been more than five years, and we ship our own lld regardlessly.
This should be happily removed.

See also:

In LLVM v9, lld has supported @response-file
LLVM v9 was released on 2019-09-19.
And the check was added back to 2018-03-14 (1.26.0) via 04442af.
It has been more than five years, and we ship our own lld regardlessly.
This should be happily removed.

See also:

* <llvm/llvm-project@bb12396>
* <https://reviews.llvm.org/D63024>
@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 12, 2025
@weihanglo
Copy link
Member Author

r? @jieyouxu

as you responded in #138421. Feel free to reassign :)

@rustbot rustbot assigned jieyouxu and unassigned fee1-dead Mar 12, 2025
@jieyouxu
Copy link
Member

It makes sense to me, however I'm no LLD expert, so maybe r? @lqd who might know a bit more about LLD than me :D

@rustbot rustbot assigned lqd and unassigned jieyouxu Mar 12, 2025
@lqd
Copy link
Member

lqd commented Mar 13, 2025

It has been more than five years, and we ship our own lld regardlessly.

This feels OK to me in principle as well, so I'm sympathetic to the first half of this sentence. However, we have to support the llds people use, so the latter half is less relevant to the PR.

The last time I checked our benchmarks, lld on windows wasn't particularly faster than msvc's link, so that configuration may not be particularly common, in addition to being fixed upstream already.

Given all this, I think we should do this. It seems highly unlikely that people would open issues and see this as a blocking in the wild, given the easy fix of not using an ancient version of lld with a recent version of rustc, and the fact that using lld and rust-lld is even easier on windows than on linux.

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 13, 2025

📌 Commit c8a6fcc has been approved by lqd

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 13, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e928a8f into rust-lang:master Mar 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup merge of rust-lang#138432 - weihanglo:lld, r=lqd

fix: remove the check of lld not supporting @response-file

In LLVM v9, lld has supported `@response-file.`
LLVM v9 was released on 2019-09-19.
The check was added back to 2018-03-14 (1.26.0) via 04442af.
It has been more than five years, and we ship our own lld regardlessly.
This should be happily removed.

See also:

* <llvm/llvm-project@bb12396>
* <https://reviews.llvm.org/D63024>
@weihanglo weihanglo deleted the lld branch March 14, 2025 11:23
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
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.

6 participants