Skip to content

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

spastorino and others added 11 commits July 20, 2023 22:20
Drop duplicate helper methods on `Layout`, which are already implemented
on `LayoutS`. Note that `Layout` has a `Deref` implementation to
`LayoutS`, so all accessors are automatically redirected.

The methods are identical and have been copied to `rustc_abi` in:

    commit 390a637
    Author: hamidreza kalbasi <[email protected]>
    Date:   Mon Nov 7 00:36:11 2022 +0330

        move things from rustc_target::abi to rustc_abi

This commit left behind the original implementation. Drop it now.

Signed-off-by: David Rheinsberg <[email protected]>
Include the computed alignment of the violating field when rejecting
transparent types with non-trivially aligned ZSTs.

ZST member fields in transparent types must have an alignment of 1 (to
ensure it does not raise the layout requirements of the transparent
field). The current error message looks like this:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment larger than 1

This patch changes the report to include the alignment of the violating
field:

 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment of 4, which is larger than 1

In case of unknown alignments, it will yield:

 LL | struct Foobar<T>(u32, [T; 0]);
    |                       ^^^^^^ may have alignment larger than 1

This allows developers to get a better grasp why a specific field is
rejected. Knowing the alignment of the violating field makes it easier
to judge where that alignment-requirement originates, and thus hopefully
provide better hints on how to mitigate the problem.

This idea was proposed in 2022 in rust-lang#98071 as part of a bigger change.
This commit simply extracts this error-message change, to decouple it
from the other diagnostic improvements.
…rrors

new solver: add a separate cache for coherence

based on rust-lang#113835

`typenum` now compiles with `-Ztrait-solver=next-coherence`. Because we disable caching once we encounter the recursion limit it is still really slow, but at least it passes and I will deal with overflow handling afterwards.

`typenum` previously hanged. The top 10 goals were
```rust
1749948 counts
(  1)   601056 (34.3%, 34.3%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [^1_0], def_id: DefId(0:1196 ~ typenum[0ab6]::type_operators::Len::Output) }, Term::Ty(^1_1)), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }, CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  2)   240422 (13.7%, 48.1%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [<^1_0 as type_operators::Len>::Output, bit::B1], def_id: DefId(1:2752 ~ core[a405]::ops::arith::Add::Output) }, Term::Ty(^1_1)), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }, CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  3)   198367 (11.3%, 59.4%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<^1_0 as core::marker::Sized>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  4)    96168 ( 5.5%, 64.9%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [<^1_0 as type_operators::Len>::Output, bit::B1], def_id: DefId(1:2752 ~ core[a405]::ops::arith::Add::Output) }, Term::Ty(<<^1_0 as type_operators::Len>::Output as core::ops::Add<bit::B1>>::Output)), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  5)    78155 ( 4.5%, 69.4%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<^1_0 as marker_traits::Unsigned>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  6)    72140 ( 4.1%, 73.5%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<^1_0 as marker_traits::Bit>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  7)    60107 ( 3.4%, 76.9%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<^1_0 as type_operators::Len>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  8)    60106 ( 3.4%, 80.4%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [uint::UInt<^1_0, ^1_1>], def_id: DefId(0:1196 ~ typenum[0ab6]::type_operators::Len::Output) }, Term::Ty(^1_2)), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }, CanonicalVarInfo { kind: Ty(General(U0)) }, CanonicalVarInfo { kind: Ty(General(U0)) }] }
(  9)    60106 ( 3.4%, 83.8%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<<<^1_0 as type_operators::Len>::Output as core::ops::Add<bit::B1>>::Output as marker_traits::Unsigned>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
( 10)    60106 ( 3.4%, 87.2%): Canonical { value: QueryInput { goal: Goal { predicate: Binder { value: TraitPredicate(<<^1_0 as type_operators::Len>::Output as core::ops::Add<bit::B1>>, polarity:Positive), bound_vars: [] }, param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst } }, anchor: Bubble, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [CanonicalVarInfo { kind: Ty(General(U0)) }] }
```

r? ``@compiler-errors``
error/E0691: include alignment in error message

Include the computed alignment of the violating field when rejecting transparent types with non-trivially aligned ZSTs.

ZST member fields in transparent types must have an alignment of 1 (to ensure it does not raise the layout requirements of the transparent field). The current error message looks like this:

```text
 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment larger than 1
```

This patch changes the report to include the alignment of the violating field:

```text
 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ has alignment of 4, which is larger than 1
```

In case of unknown alignments, it will yield:

```text
 LL | struct Foobar(u32, [u32; 0]);
    |                    ^^^^^^^^ may have alignment larger than 1
```

This allows developers to get a better grasp why a specific field is rejected. Knowing the alignment of the violating field makes it easier to judge where that alignment-requirement originates, and thus hopefully provide better hints on how to mitigate the problem.

This idea was proposed in 2022 in rust-lang#98071 as part of a bigger change. This commit simply extracts this error-message change, to decouple it from the other diagnostic improvements.

(Originally proposed by `@compiler-errors` in rust-lang#98071)
rustc_target: drop duplicate code

Drop duplicate helper methods on `Layout`, which are already implemented on `LayoutS`. Note that `Layout` has a `Deref` implementation to `LayoutS`, so all accessors are automatically redirected.

The methods are identical and have been copied to `rustc_abi` in:

    commit 390a637
    Author: hamidreza kalbasi <[email protected]>
    Date:   Mon Nov 7 00:36:11 2022 +0330

        move things from rustc_target::abi to rustc_abi

This commit left behind the original implementation. Drop it now.

(originally moved by ``@HKalbasi)``
@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Jul 21, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Collaborator

bors commented Jul 21, 2023

📌 Commit 1aaf583 has been approved by matthiaskrgr

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 Jul 21, 2023
@bors
Copy link
Collaborator

bors commented Jul 21, 2023

⌛ Testing commit 1aaf583 with merge c3c5a5c...

@bors
Copy link
Collaborator

bors commented Jul 21, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing c3c5a5c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 21, 2023
@bors bors merged commit c3c5a5c into rust-lang:master Jul 21, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 21, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#113887 new solver: add a separate cache for coherence 587ebe64df410883dc8626eb24549bd5765d97b3 (link)
#113910 Add FnPtr ty to SMIR 4ec10499a9e7ece50222b1435c24b90b3f09c1fd (link)
#113913 error/E0691: include alignment in error message e24f7c66104e8eba342870403dbe953ba4a0b87d (link)
#113914 rustc_target: drop duplicate code f30a075d6f57c9fd9dc3c386d4ff415b767053a5 (link)

previous master: 557359f925

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c3c5a5c): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.6% [-3.9%, -2.9%] 6
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -3.6% [-3.9%, -2.9%] 6

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 658.335s -> 658.525s (0.03%)

@matthiaskrgr matthiaskrgr deleted the rollup-90cj2vv branch March 16, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants