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 7 pull requests #94357

Merged
merged 15 commits into from
Feb 25, 2022
Merged

Rollup of 7 pull requests #94357

merged 15 commits into from
Feb 25, 2022

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

lcnr and others added 15 commits February 24, 2022 14:46
…r=cjgillot

Remove in band lifetimes

As discussed in t-lang backlog bonanza, the `in_band_lifetimes` FCP closed in favor for the feature not being stabilized. This PR removes `#![feature(in_band_lifetimes)]` in its entirety.

Let me know if this PR is too hasty, and if we should instead do something intermediate for deprecate the feature first.

r? `@scottmcm` (or feel free to reassign, just saw your last comment on rust-lang#44524)
Closes rust-lang#44524
…st, r=jsha

Extend toggle GUI test a bit

Fixes rust-lang#84422.

r? `@jsha`
don't special case `DefKind::Ctor` in encoding

considering that we still use `DefKind::Ctor` for these in `Res`, this seems weird and definitely felt like a bug when encountering it while working on rust-lang#89862.

r? `@cjgillot`
Remove an unnecessary restriction in `dest_prop`

I had asked about this [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Do.20unions.20have.20active.20fields.3F) but didn't receive a response, so putting up this PR that makes the change I think we can. If it turns out that this is wrong, hopefully I'll find out here. Reposting my Zulip comment:
> Not sure what channel to put this into, so using this as a fallback. The dest prop MIR opt has this comment:
>
> ```rust
> //!   Subtle case: If `dest` is a, or projects through a union, then we have to make sure that there
> //!   remains an assignment to it, since that sets the "active field" of the union. But if `src` is
> //!   a ZST, it might not be initialized, so there might not be any use of it before the assignment,
> //!   and performing the optimization would simply delete the assignment, leaving `dest`
> //!   uninitialized.
> ```
>
> In particular, the claim seems to be that we can't take
> ```
> x = ();
> y.field = x;
> ```
> where `y` is a union having `field: ()` as one of its variants, and optimize the entire thing away (assuming `x` is unused otherwise). As far as I know though, Rust unions don't have active fields. Is this comment correct and am I missing something? Is there a worry about this interacting poorly with FFI code/C unions/LTO or something?

This PR just removes that comment and the associated code. Also it fixes one unrelated comment that did not match the code it was commenting on.

r? rust-lang/mir-opt
Miri fn ptr check: don't use conservative null check

In rust-lang#94270 I used the wrong NULL check for function pointers: `memory.ptr_may_be_null` is conservative even on machines that support ptr-to-int casts, leading to false errors in Miri.

This fixes that problem, and also replaces that foot-fun of a method with `scalar_may_be_null` which is never unnecessarily conservative.

r? `@oli-obk`
…more, r=oli-obk

diagnostic: suggest parens when users want logical ops, but get closures

Fixes rust-lang#93536
…-Simulacrum

Fix SGX docs build

Without this, I get
```
error[E0432]: unresolved import `crate::sys::cvt`
  --> library/std/src/os/fd/owned.rs:12:5
   |
12 | use crate::sys::cvt;
   |     ^^^^^^^^^^^^^^^ no `cvt` in `sys`
```
when running rustdoc on `std` for the x86_64-fortanix-unknown-sgx target.
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Feb 25, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Feb 25, 2022

📌 Commit 6060645 has been approved by matthiaskrgr

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 25, 2022
@bors
Copy link
Contributor

bors commented Feb 25, 2022

⌛ Testing commit 6060645 with merge 9f8f0a6...

@bors
Copy link
Contributor

bors commented Feb 25, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 25, 2022
@bors bors merged commit 9f8f0a6 into rust-lang:master Feb 25, 2022
@rustbot rustbot added this to the 1.61.0 milestone Feb 25, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9f8f0a6): comparison url.

Summary: This benchmark run shows 4 relevant regressions 😿 to instruction counts.

  • Arithmetic mean of relevant regressions: 0.8%
  • Largest regression in instruction counts: 0.9% on incr-unchanged builds of externs debug

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Feb 25, 2022
@Mark-Simulacrum
Copy link
Member

Dropping the regression label -- externs is currently a little noisy (and we haven't absorbed that noisiness into our statistical estimation yet). Seems to be due to #93839, which makes me suspect this is related to PGO or inlining decisions of some kind. #94373 might help.

@Mark-Simulacrum Mark-Simulacrum removed the perf-regression Performance regression. label Mar 1, 2022
@matthiaskrgr matthiaskrgr deleted the rollup-xrjaof3 branch March 11, 2022 15:32
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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.