[beta] Backports #47682
Merged
[beta] Backports #47682
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The match lowering code, when lowering matches against bytestrings, works by coercing both the scrutinee and the pattern to `&[u8]` and then comparing them using `<[u8] as Eq>::eq`. If the scrutinee is already of type `&[u8]`, then unsizing it is both unneccessary and a trait error caught by the new and updated MIR typeck, so this PR changes lowering to avoid doing that (match lowering tried to avoid that before, but that attempt was quite broken). Fixes rust-lang#46920.
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
|
|
|
Thanks! It looks like the "lower link args" isn't cleanly backporting (build failure), mind fixing that up? If you're busy though I can take a look! |
When spawning a linker rustc has historically been known to blow OS limits for the command line being too large, notably on Windows. This is especially true of incremental compilation where there can be dozens of object files per compilation. The compiler currently has logic for detecting a failure to spawn and instead passing arguments via a file instead, but this failure detection only triggers if a process actually fails to spawn. Unfortunately on Windows we've got something else to worry about which is `cmd.exe`. The compiler may be running a linker through `cmd.exe` where `cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`. Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently detect. Consequently this commit updates the logic for the spawning the linker on Windows to instead have a heuristic to see if we need to pass arguments via a file. This heuristic is an overly pessimistic and "inaccurate" calculation which just calls `len` on a bunch of `OsString` instances (where `len` is not precisely the length in u16 elements). This number, when exceeding the 6k threshold, will force rustc to always pass arguments through a file. This strategy should avoid us trying to parse the output on Windows of the linker to see if it successfully spawned yet failed to actually sub-spawn the linker. We may just be passing arguments through files a little more commonly now... The motivation for this commit was a recent bug in Gecko [1] when beta testing, notably when incremental compilation was enabled it blew out the limit on `cmd.exe`. This commit will also fix rust-lang#46999 as well though as emscripten uses a bat script as well (and we're blowing the limit there). [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886 Closes rust-lang#46999 (cherry picked from commit 66366f9)
|
Thanks @alexcrichton for this info, I will take a look at it now! |
kennytm
added
the
S-waiting-on-author
MaloJaffre
force-pushed
the
beta-backport
branch
from
e73f5dc
to
590924b
Compare
kennytm
added
S-waiting-on-review
|
The build should be fixed now. |
|
@bors: r+ p=5 Thanks! |
|
📌 Commit 590924b has been approved by |
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picked into beta:
@-files on Windows more #47507