Rollup of 8 pull requests #144313
Closed
Rollup of 8 pull requests #144313
Conversation
This file contains hidden or 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
…nd the LLVM codegen bckend.
On the 32-bit win7 target, we use OS TLS instead of native TLS, due to issues with how the OS handles alignment. Unfortunately, this caused issues due to the TLS destructors not running, causing memory leaks among other problems. On Windows, to support OS TLS, the TlsAlloc family of function is used by Rust. This function does not support TLS destructors at all. However, rust has some code to emulate those destructors, by leveraging the TLS support functionality found in the MSVC CRT (specifically, in tlssup.c of the CRT). Specifically, the CRT provides the ability to register callbacks that are called (among other things) on thread destruction. By registering our own callback, we can run through a list of registered destructors functions to execute. To use this functionality, the user must do two things: 1. They must put the address to their callback in a section between `.CRT$XLB` and `.CRT$XLY`. 2. They must add a reference to `_tls_used` (or `__tls_used` on x86) to make sure the TLS support code in tlssup.c isn't garbage collected by the linker. Prior to this commit, this second bit wasn't being done properly by the Rust TLS support code. Instead of adding a reference to _tls_used, it instead had a reference to its own callback to prevent it from getting GC'd by the linker. While this is _also_ necessary, not having a reference on _tls_used made the entire support non-functional. This commit reworks the code to: 1. Add an unconditional `#[used]` attribute on the CALLBACK, which should be enough to prevent it from getting GC'd by the linker. 2. Add a reference to `_tls_used`, which should pull the TLS support code into the Rust programs and not let it be GC'd by the linker.
The previous manual parsing of `serde_json::Value` was a lot of
complicated code and extremely error-prone. It was full of janky
behavior like sometimes ignoring type errors, sometimes erroring for
type errors, sometimes warning for type errors, and sometimes just
ICEing for type errors (the icing on the top).
Additionally, many of the error messages about allowed values were out
of date because they were in a completely different place than the
FromStr impls. Overall, the system caused confusion for users.
I also found the old deserialization code annoying to read. Whenever a
`key!` invocation was found, one had to first look for the right macro
arm, and no go to definition could help.
This PR replaces all this manual parsing with a 2-step process involving
serde.
First, the string is parsed into a `TargetSpecJson` struct. This struct
is a 1:1 representation of the spec JSON. It already parses all the
enums and is very simple to read and write.
Then, the fields from this struct are copied into the actual `Target`.
The reason for this two-step process instead of just serializing into a
`Target` is because of a few reasons
1. There are a few transformations performed between the two formats
2. The default logic is implemented this way. Otherwise all the default
field values would have to be spelled out again, which is
suboptimal. With this logic, they fall out naturally, because
everything in the json struct is an `Option`.
Overall, the mapping is pretty simple, with the vast majority of fields
just doing a 1:1 mapping that is captured by two macros. I have
deliberately avoided making the macros generic to keep them simple.
All the `FromStr` impls now have the error message right inside them,
which increases the chance of it being up to date. Some "`from_str`"
impls were turned into proper `FromStr` impls to support this.
The new code is much less involved, delegating all the JSON parsing
logic to serde, without any manual type matching.
This change introduces a few breaking changes for consumers. While it is
possible to use this format on stable, it is very much subject to
change, so breaking changes are expected. The hope is also that because
of the way stricter behavior, breaking changes are easier to deal with,
as they come with clearer error messages.
1. Invalid types now always error, everywhere. Previously, they would
sometimes error, and sometimes just be ignored (which meant the users
JSON was still broken, just silently!)
2. This now makes use of `deny_unknown_fields` instead of just warning
on unused fields, which was done previously. Serde doesn't make it
easy to get such warning behavior, which was the primary reason that
this now changed. But I think error behavior is very reasonable too.
If someone has random stale fields in their JSON, it is likely
because these fields did something at some point but no longer do,
and the user likely wants to be informed of this so they can figure
out what to do.
This is also relevant for the future. If we remove a field but
someone has it set, it probably makes sense for them to take a look
whether they need this and should look for alternatives, or whether
they can just delete it. Overall, the JSON is made more explicit.
This is the only expected breakage, but there could also be small
breakage from small mistakes. All targets roundtrip though, so it can't
be anything too major.
…trochenkov Ensure we codegen the main fn This fixes two bugs. The one that was identified in the linked issue is that when we have a `main` function, mono collection didn't consider it as an extra collection root. The other is that since CGU partitioning doesn't know about the call edges between the entrypoint functions, naively it can put them in different CGUs and mark them all as internal. Which would result in LLVM just deleting all of them. There was an existing hack to exclude `lang = "start"` from internalization, which I've extended to include `main`. Fixes rust-lang#144052
Remove tidy checks for `tests/ui/issues/` r? ``@jieyouxu`` As it is making cleanup efforts more difficult. This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
…, r=fee1-dead Use serde for target spec json deserialize The previous manual parsing of `serde_json::Value` was a lot of complicated code and extremely error-prone. It was full of janky behavior like sometimes ignoring type errors, sometimes erroring for type errors, sometimes warning for type errors, and sometimes just ICEing for type errors (the icing on the top). Additionally, many of the error messages about allowed values were out of date because they were in a completely different place than the FromStr impls. Overall, the system caused confusion for users. I also found the old deserialization code annoying to read. Whenever a `key!` invocation was found, one had to first look for the right macro arm, and no go to definition could help. This PR replaces all this manual parsing with a 2-step process involving serde. First, the string is parsed into a `TargetSpecJson` struct. This struct is a 1:1 representation of the spec JSON. It already parses all the enums and is very simple to read and write. Then, the fields from this struct are copied into the actual `Target`. The reason for this two-step process instead of just serializing into a `Target` is because of a few reasons 1. There are a few transformations performed between the two formats 2. The default logic is implemented this way. Otherwise all the default field values would have to be spelled out again, which is suboptimal. With this logic, they fall out naturally, because everything in the json struct is an `Option`. Overall, the mapping is pretty simple, with the vast majority of fields just doing a 1:1 mapping that is captured by two macros. I have deliberately avoided making the macros generic to keep them simple. All the `FromStr` impls now have the error message right inside them, which increases the chance of it being up to date. Some "`from_str`" impls were turned into proper `FromStr` impls to support this. The new code is much less involved, delegating all the JSON parsing logic to serde, without any manual type matching. This change introduces a few breaking changes for consumers. While it is possible to use this format on stable, it is very much subject to change, so breaking changes are expected. The hope is also that because of the way stricter behavior, breaking changes are easier to deal with, as they come with clearer error messages. 1. Invalid types now always error, everywhere. Previously, they would sometimes error, and sometimes just be ignored (which meant the users JSON was still broken, just silently!) 2. This now makes use of `deny_unknown_fields` instead of just warning on unused fields, which was done previously. Serde doesn't make it easy to get such warning behavior, which was the primary reason that this now changed. But I think error behavior is very reasonable too. If someone has random stale fields in their JSON, it is likely because these fields did something at some point but no longer do, and the user likely wants to be informed of this so they can figure out what to do. This is also relevant for the future. If we remove a field but someone has it set, it probably makes sense for them to take a look whether they need this and should look for alternatives, or whether they can just delete it. Overall, the JSON is made more explicit. This is the only expected breakage, but there could also be small breakage from small mistakes. All targets roundtrip though, so it can't be anything too major. fixes rust-lang#144153
generate elf symbol version in raw-dylib For link names like `aaa@bbb`, it generates a symbol named `aaa` and a version named `bbb`. For link names like `aaa\0bbb`, `aaa@``@bbb``` or `aa@bb@cc`, it emits errors. It adds a test that the executable is linked with glibc using raw-dylib. cc rust-lang#135694
…-obk Implement support for `become` and explicit tail call codegen for the LLVM backend This PR implements codegen of explicit tail calls via `become` in `rustc_codegen_ssa` and support within the LLVM backend. Completes a task on (rust-lang#112788). This PR implements all the necessary bits to make explicit tail calls usable, other backends have received stubs for now and will ICE if you use `become` on them. I suspect there is some bikeshedding to be done on how we should go about implementing this for other backends, but it should be relatively straightforward for GCC after this is merged. During development I also put together a POC bytecode VM based on tail call dispatch to test these changes out and analyze the codegen to make sure it generates expected assembly. That is available [here](https://github.com/xacrimon/tcvm).
…isDenton Fix broken TLS destructors on 32-bit win7 Fixes rust-lang#141300 On the 32-bit win7 target, we use OS TLS instead of native TLS, due to issues with how the OS handles alignment. Unfortunately, this caused issues due to the TLS destructors not running, causing memory leaks among other problems. On Windows, to support OS TLS, the TlsAlloc family of function is used by Rust. This function does not support TLS destructors at all. However, rust has some code to emulate those destructors, by leveraging the TLS support functionality found in the MSVC CRT (specifically, in tlssup.c of the CRT). To use this functionality, the user must do two things: 1. They must put the address to their callback in a section between `.CRT$XLB` and `.CRT$XLY`. 2. They must add a reference to `_tls_used` (or `__tls_used` on x86) to make sure the TLS support code in tlssup.c isn't garbage collected by the linker. Prior to this commit, this second bit wasn't being done properly by the Rust TLS support code. Instead of adding a reference to _tls_used, it instead had a reference to its own callback to prevent it from getting GC'd by the linker. While this is _also_ necessary, not having a reference on _tls_used made the entire support non-functional. This commit reworks the code to: 1. Add an unconditional `#[used]` attribute on the CALLBACK, which should be enough to prevent it from getting GC'd by the linker. 2. Add a reference to `_tls_used`, which should pull the TLS support code into the Rust programs and not let it be GC'd by the linker.
Don't ICE on non-TypeId metadata within TypeId fixes rust-lang#144253 r? ``@RalfJung``
…enkov Use less HIR in check_private_in_public. r? ``@petrochenkov``
rustbot
added
A-LLVM
rustbot
added
T-compiler
|
@bors r+ rollup=never p=5 |
bors
added
S-waiting-on-bors
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.
Successful merges:
tests/ui/issues/#144173 (Remove tidy checks fortests/ui/issues/)becomeand explicit tail call codegen for the LLVM backend #144232 (Implement support forbecomeand explicit tail call codegen for the LLVM backend)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup