Currently, deriving on packed structs has some non-trivial limitations,
related to the fact that taking references on unaligned fields is UB.
The current approach to field accesses in derived code:
- Normal case: `&self.0`
- In a packed struct that derives `Copy`: `&{self.0}`
- In a packed struct that doesn't derive `Copy`: `&self.0`
Plus, we disallow deriving any builtin traits other than `Default` for any
packed generic type, because it's possible that there might be
misaligned fields. This is a fairly broad restriction.
Plus, we disallow deriving any builtin traits other than `Default` for most
packed types that don't derive `Copy`. (The exceptions are those where the
alignments inherently satisfy the packing, e.g. in a type with
`repr(packed(N))` where all the fields have alignments of `N` or less
anyway. Such types are pretty strange, because the `packed` attribute is
not having any effect.)
This commit introduces a new, simpler approach to field accesses:
- Normal case: `&self.0`
- In a packed struct: `&{self.0}`
In the latter case, this requires that all fields impl `Copy`, which is
a new restriction. This means that the following example compiles under
the old approach and doesn't compile under the new approach.
```
#[derive(Debug)]
struct NonCopy(u8);
#[derive(Debug)
#[repr(packed)]
struct MyType(NonCopy);
```
(Note that the old approach's support for cases like this was brittle.
Changing the `u8` to a `u16` would be enough to stop it working. So not
much capability is lost here.)
However, the other constraints from the old rules are removed. We can now
derive builtin traits for packed generic structs like this:
```
trait Trait { type A; }
#[derive(Hash)]
#[repr(packed)]
pub struct Foo<T: Trait>(T, T::A);
```
To allow this, we add a `T: Copy` bound in the derived impl and a `T::A:
Copy` bound in where clauses. So `T` and `T::A` must impl `Copy`.
We can now also derive builtin traits for packed structs that don't derive
`Copy`, so long as the fields impl `Copy`:
```
#[derive(Hash)]
#[repr(packed)]
pub struct Foo(u32);
```
This includes types that hand-impl `Copy` rather than deriving it, such as the
following, that show up in winapi-0.2:
```
#[derive(Clone)]
#[repr(packed)]
struct MyType(i32);
impl Copy for MyType {}
```
The new approach is simpler to understand and implement, and it avoids
the need for the `unsafe_derive_on_repr_packed` check.
One exception is required for backwards-compatibility: we allow `[u8]`
fields for now. There is a new lint for this,
`byte_slice_in_packed_struct_with_derive`.
Port pgo.sh to Python
This PR ports the `pgo.sh` multi stage build file from bash to Python, to make it easier to add new functionality and gather statistics. Main changes:
1) `pgo.sh` rewritten from Bash to Python. Jump from ~200 Bash LOC to ~650 Python LOC. Bash is, unsurprisingly, more concise for running scripts and binaries.
2) Better logging. Each separate stage is now clearly separated in logs, and the logs can be quickly grepped to find out which stage has completed or failed, and how long it took.
3) Better statistics. At the end of the run, there is now a table that shows the duration of the individual stages, along with a percentual ratio of the total workflow run:
```
2023-01-15T18:13:49.9896916Z stage-build INFO: Timer results
2023-01-15T18:13:49.9902185Z ---------------------------------------------------------
2023-01-15T18:13:49.9902605Z Build rustc (LLVM PGO): 1815.67s (21.47%)
2023-01-15T18:13:49.9902949Z Gather profiles (LLVM PGO): 418.73s ( 4.95%)
2023-01-15T18:13:49.9903269Z Build rustc (rustc PGO): 584.46s ( 6.91%)
2023-01-15T18:13:49.9903835Z Gather profiles (rustc PGO): 806.32s ( 9.53%)
2023-01-15T18:13:49.9904154Z Build rustc (LLVM BOLT): 1662.92s (19.66%)
2023-01-15T18:13:49.9904464Z Gather profiles (LLVM BOLT): 715.18s ( 8.46%)
2023-01-15T18:13:49.9914463Z Final build: 2454.00s (29.02%)
2023-01-15T18:13:49.9914798Z Total duration: 8457.27s
2023-01-15T18:13:49.9915305Z ---------------------------------------------------------
```
A sample run can be seen [here](https://github.com/rust-lang/rust/actions/runs/3923980164/jobs/6707932029).
I tried to keep the code compatible with Python 3.6 and don't use dependencies, which required me to reimplement some small pieces of functionality (like formatting bytes). I suppose that it shouldn't be so hard to upgrade to a newer Python or install dependencies in the CI container, but I'd like to avoid it if it won't be needed.
The code is in a single file `stage-build.py`, so it's a bit cluttered. I can also separate it into multiple files, although having it in a single file has some benefits. The code could definitely be nicer, but I'm a bit wary of introducing a lot of abstraction and similar stuff, as long as the code is stuffed into a single file.
Currently, the Python pipeline should faithfully mirror the bash pipeline one by one. After this PR, I'd like to try to optimize it, e.g. by caching the LLVM builds on S3.
r? `@Mark-Simulacrum`
Rollup of 11 pull requests
Successful merges:
- #96763 (Fix maintainer validation message)
- #106540 (Insert whitespace to avoid ident concatenation in suggestion)
- #106763 (print why a test was ignored if its the only test specified)
- #106769 (libtest: Print why a test was ignored if it's the only test specified.)
- #106798 (Implement `signum` with `Ord`)
- #107006 (Output tree representation on thir-tree)
- #107078 (Update wording of invalid_doc_attributes docs.)
- #107169 (Pass `--locked` to the x test tidy call)
- #107431 (docs: remove colon from time header)
- #107432 (rustdoc: remove unused class `has-srclink`)
- #107448 (When stamp doesn't exist, should say Error, and print path to stamp file)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Update wording of invalid_doc_attributes docs.
There was a typo in the original docs for `invalid_doc_attributes`. I felt it could use a little rewording to try to clarify the reasoning for the lint. Also, this adds the future-incompatible notice.
Output tree representation on thir-tree
The current output of `-Zunpretty=thir-tree` is really cumbersome to work with, using an actual tree representation should make it easier to see what the thir looks like.
Implement `signum` with `Ord`
Rather than needing to do things like #105840 for `signum` too, might as well just implement that method using `Ord`, since it's doing the same "I need `-1`/`0`/`+1`" behaviour that `cmp` is already doing.
This also seems to slightly improve the assembly: <https://rust.godbolt.org/z/5oEEqbxK1>
Insert whitespace to avoid ident concatenation in suggestion
This PR tweaks the suggestion of removing misplaced parentheses around trait bounds so as to avoid concatenating two identifiers. Although subtle, this should make outputs less surprising especially when applying the `MachineApplicable` suggestions automatically.
Fix maintainer validation message
State clearly why maintainer verification is not activating, ruling out the user thinking they misconfigured something.
Implement simple CopyPropagation based on SSA analysis
This PR extracts the "copy propagation" logic from https://github.com/rust-lang/rust/pull/106285.
MIR may produce chains of assignment between locals, like `_x = move? _y`.
This PR attempts to remove such chains by unifying locals.
The current implementation is a bit overzealous in turning moves into copies, and in removing storage statements.
Skip possible where_clause_object_safety lints when checking `multiple_supertrait_upcastable`
Fix#106247
To achieve this, I lifted the `WhereClauseReferencesSelf` out from `object_safety_violations` and move it into `is_object_safe` (which is changed to a new query).
cc `@dtolnay`
r? `@compiler-errors`
Rollup of 8 pull requests
Successful merges:
- #106618 (Disable `linux_ext` in wasm32 and fortanix rustdoc builds.)
- #107097 (Fix def-use dominance check)
- #107154 (library/std/sys_common: Define MIN_ALIGN for m68k-unknown-linux-gnu)
- #107397 (Gracefully exit if --keep-stage flag is used on a clean source tree)
- #107401 (remove the usize field from CandidateSource::AliasBound)
- #107413 (make more pleasant to read)
- #107422 (Also erase substs for new infcx in pin move error)
- #107425 (Check for missing space between fat arrow and range pattern)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Check for missing space between fat arrow and range pattern
Fixes#107420
Ideally we wouldn't emit an error about expecting `=>` etc., but I'm not sure how to recover from this.
`@rustbot` label +A-diagnostics
Also erase substs for new infcx in pin move error
The code originally correctly erased the regions of the type it passed to the newly created infcx. But after the `fn_sig` query was made to return an `EarlyBinder<T>`, some substs that were around were substituted there without erasing their regions. They were then passed into the newly cerated infcx, which caused the ICE.
Fixes#107419
r? compiler-errors who reviewed the original PR adding this diagnostic
Gracefully exit if --keep-stage flag is used on a clean source tree
Instead of quitting with an obscure No such file or directory error, give the user a clearer and easier to understand error (as well as suggesting a possible cause for the error).
This is the first time I have written rust since 2020, and the first PR I will ever make to rust, so please do point out any mistakes I have made 😄 .
This fixes#107392
library/std/sys_common: Define MIN_ALIGN for m68k-unknown-linux-gnu
This PR adds the missing definition of MIN_ALIGN for the m68k-unknown-linux target.
Fix def-use dominance check
A definition does not dominate a use in the same statement. For example
in MIR generated for compound assignment x += a (when overflow checks
are disabled).
Disable `linux_ext` in wasm32 and fortanix rustdoc builds.
The `std::os::unix` module is stubbed out when building docs for these target platforms. The introduction of Linux-specific extension traits caused `std::os::net` to depend on sub-modules of `std::os::unix`, which broke rustdoc for the `wasm32-unknown-unknown` target.
Adding an additional `#[cfg]` guard solves that rustdoc failure by not declaring `linux_ext` on targets with a stubbed `std::os::unix`.
Fixes#105467
Use stable metric for const eval limit instead of current terminator-based logic
This patch adds a `MirPass` that inserts a new MIR instruction `ConstEvalCounter` to any loops and function calls in the CFG. This instruction is used during Const Eval to count against the `const_eval_limit`, and emit the `StepLimitReached` error, replacing the current logic which uses Terminators only.
The new method of counting loops and function calls should be more stable across compiler versions (i.e., not cause crates that compiled successfully before, to no longer compile when changes to the MIR generation/optimization are made).
Also see: #103877
Special-case deriving `PartialOrd` for enums with dataless variants
I was able to get slightly better codegen by flipping the derived `PartialOrd` logic for two-variant enums. I also tried to document the implementation of the derive macro to make the special-case logic a little clearer.
```rs
#[derive(PartialEq, PartialOrd)]
pub enum A<T> {
A,
B(T)
}
```
```diff
impl<T: ::core::cmp::PartialOrd> ::core::cmp::PartialOrd for A<T> {
#[inline]
fn partial_cmp(
&self,
other: &A<T>,
) -> ::core::option::Option<::core::cmp::Ordering> {
let __self_tag = ::core::intrinsics::discriminant_value(self);
let __arg1_tag = ::core::intrinsics::discriminant_value(other);
- match ::core::cmp::PartialOrd::partial_cmp(&__self_tag, &__arg1_tag) {
- ::core::option::Option::Some(::core::cmp::Ordering::Equal) => {
- match (self, other) {
- (A::B(__self_0), A::B(__arg1_0)) => {
- ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0)
- }
- _ => ::core::option::Option::Some(::core::cmp::Ordering::Equal),
- }
+ match (self, other) {
+ (A::B(__self_0), A::B(__arg1_0)) => {
+ ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0)
}
- cmp => cmp,
+ _ => ::core::cmp::PartialOrd::partial_cmp(&__self_tag, &__arg1_tag),
}
}
}
```
Godbolt: [Current](https://godbolt.org/z/GYjEzG1T8), [New](https://godbolt.org/z/GoK78qx15)
I'm not sure how common a case comparing two enums like this (such as `Option`) is, and if it's worth the slowdown of adding a special case to the derive. If it causes overall regressions it might be worth just manually implementing this for `Option`.
