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

Rollup of 8 pull requests #107400

Merged
merged 21 commits into from
Jan 28, 2023
Merged

Rollup of 8 pull requests #107400

merged 21 commits into from
Jan 28, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

scottmcm and others added 21 commits January 18, 2023 19:19
…u-se

Implement `SpecOptionPartialEq` for `cmp::Ordering`

Noticed as I continue to explore options for having code using `partial_cmp` optimize better.

Before:
```llvm
; Function Attrs: mustprogress nofree nosync nounwind willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #0 {
start:
  %2 = icmp eq i8 %0, 2
  br i1 %2, label %bb1.i, label %bb3.i

bb1.i:                                            ; preds = %start
  %3 = icmp eq i8 %1, 2
  br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"

bb3.i:                                            ; preds = %start
  %.not.i = icmp ne i8 %1, 2
  %4 = icmp eq i8 %0, %1
  %spec.select.i = and i1 %.not.i, %4
  br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"

"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit": ; preds = %bb1.i, %bb3.i
  %.0.i = phi i1 [ %3, %bb1.i ], [ %spec.select.i, %bb3.i ]
  ret i1 %.0.i
}
```

After:
```llvm
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #1 {
start:
  %2 = icmp eq i8 %0, %1
  ret i1 %2
}
```

(Which <https://alive2.llvm.org/ce/z/-rop5r> says LLVM *could* just do itself, but there's probably an issue already open for that problem from when this was originally looked at for `Option<NonZeroU8>` and friends.)
Use proper `InferCtxt` when probing for associated types in astconv

Fixes rust-lang#107087
…e_obligation, r=lcnr

Use new solver in `evaluate_obligation` query (when new solver is enabled)

(only when `-Ztrait-solver=next`, of course)

... Does this make sense? It seems to me like it should be reasonable, but maybe there's some reason why this is a bad idea.

r? ``@lcnr``

Needs a perf run because I guess this `solver == TraitSolver::Next` check is on a hot path.
Recover from more const arguments that are not wrapped in curly braces

Recover from some array, borrow, tuple & arithmetic expressions in const argument positions that lack curly braces and provide a suggestion to fix the issue continuing where rust-lang#92884 left off. Examples of such expressions: `[]`, `[0]`, `[1, 2]`, `[0; 0xff]`, `&9`, `("", 0)` and `(1 + 2) * 3` (we previously did not recover from them).

I am not entirely happy with my current solution because the code that recovers from `[0]` (coinciding with a malformed slice type) and `[0; 0]` (coinciding with a malformed array type) is quite fragile as the aforementioned snippets are actually successfully parsed as types by `parse_ty` since it itself already recovers from them (returning `[⟨error⟩]` and `[⟨error⟩; 0]` respectively) meaning I have to manually look for `TyKind::Err`s and construct a separate diagnostic for the suggestion to attach to (thereby emitting two diagnostics in total).

Fixes rust-lang#81698.
`@rustbot` label A-diagnostics
r? diagnostics
…losure-arg-needs-borrow, r=oli-obk

Correct suggestions for closure arguments that need a borrow

Fixes rust-lang#107301 by dealing with binders correctly
Fixes another issue where we were suggesting adding just `&` when we expected `&mut _` in a closure arg
internally change regions to be covariant

Surprisingly, we consider the reference type `&'a T` to be contravaraint in its lifetime parameter. This is confusing and conflicts with the documentation we have in the reference, rustnomicon, and rustc-dev-guide. This also arguably not the correct use of terminology since we can use `&'static u8` in a place where `&' a u8` is expected, this implies that `&'static u8 <: &' a u8` and consequently `'static <: ' a`, hence covariance.

Because of this, when relating two types, we used to switch the argument positions in a confusing way:
`Subtype(&'a u8 <: &'b u8) => Subtype('b <: 'a) => Outlives('a: 'b) => RegionSubRegion('b <= 'a)`

The reason for the current behavior is probably that we wanted `Subtype('b <: 'a)` and `RegionSubRegion('b <= 'a)` to be equivalent, but I don' t think this is a good reason since these relations are sufficiently different in that the first is a relation in the subtyping lattice and is intrinsic to the type-systems, while the the second relation is an implementation detail of regionck.

This PR changes this behavior to use covariance, so..
`Subtype(&'a u8 <: &'b u8) => Subtype('a <: 'b) => Outlives('a: 'b) => RegionSubRegion('b <= 'a) `

Resolves rust-lang#103676

r? `@lcnr`
… r=lcnr

Minor tweaks in the new solver

1. `InferCtxt::probe` is not needed in `compute_subtype_goal` and `compute_well_formed_goal`.
2. Add a handful of comments.
3. Add a micro-optimization in `consider_assumption` where we check the def-ids of the assumption and goal match before instantiating any binders.

r? ``@lcnr``
…s-when-debuginfo, r=WaffleLapkin

Don't merge vtables when full debuginfo is enabled.

This PR makes the compiler not emit the `unnamed_addr` attribute for vtables when full debuginfo is enabled, so that they don't get merged even if they have the same contents. This allows debuggers to more reliably map from a dyn pointer to the self-type of a trait object by looking at the vtable's debuginfo.

The PR only changes the behavior of the LLVM backend as other backends don't emit vtable debuginfo (as far as I can tell).

The performance impact of this change should be small as [measured](rust-lang#103514 (comment)) in a previous PR.
@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative rollup A PR which is a rollup labels Jan 28, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Jan 28, 2023

📌 Commit c89bb15 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 Jan 28, 2023
@bors
Copy link
Contributor

bors commented Jan 28, 2023

⌛ Testing commit c89bb15 with merge 226b249...

@bors
Copy link
Contributor

bors commented Jan 28, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 28, 2023
@bors bors merged commit 226b249 into rust-lang:master Jan 28, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 28, 2023
@rust-timer
Copy link
Collaborator

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (226b249): 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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-6.1%, -0.7%] 4
All ❌✅ (primary) - - 0

Cycles

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

@matthiaskrgr matthiaskrgr deleted the rollup-l6bycds branch March 16, 2024 18:19
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. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants