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 5 pull requests #97347

Closed
wants to merge 16 commits into from
Closed

Conversation

Dylan-DPC
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

pvdrz and others added 16 commits May 17, 2022 13:10
this simplifies the code inside the `structure.each` closure argument
and allows to remove the `vis` field from `FieldInfo`.
(with an intermediate raw ptr, not a direct transmute)
Modify MIR building to drop repeat expressions with length zero

Closes rust-lang#74836 .

Previously, when a user wrote `[foo; 0]` we used to simply leak `foo`. The goal is to fix that. This PR changes MIR building to make `[foo; 0]` equivalent to `{ drop(foo); [] }` in all cases. Of course, this is a breaking change (see below). A crater run did not indicate any regressions though, and given that the previous behavior was almost definitely not what any user wanted, it seems unlikely that anyone was relying on this.

Note that const generics are in general unaffected by this. Inserting the extra `drop` is only meaningful/necessary when `foo` is of a non-`Copy` type, and array repeat expressions with const generic repetition count must always be `Copy`.

Besides the obvious change to behavior associated with the additional drop, there are three categories of examples where this also changes observable behavior. In all of these cases, the new behavior is consistent with what you would get by replacing `[foo; 0]` with `{ drop(foo); [] }`. As such, none of these give the user new powers to express more things.

**No longer allowed in const (breaking)**:

```rust
const _: [String; 0] = [String::new(); 0];
```

This compiles on stable today. Because we now introduce the drop of `String`, this no longer compiles as `String` may not be dropped in a const context.

**Reduced dataflow (non-breaking)**:

```rust
let mut x: i32 = 0;
let r = &x;
let a = [r; 0];
x = 5;
let _b = a;
```

Borrowck rejects this code on stable because it believes there is dataflow between `a` and `r`, and so the lifetime of `r` has to extend to the last statement. This change removes the dataflow and the above code is allowed to compile.

**More const promotion (non-breaking)**:

```rust
let _v: &'static [String; 0] = &[String::new(); 0];
```

This does not compile today because `String` having drop glue keeps it from being const promoted (despite that drop glue never being executed). After this change, this is allowed to compile.

### Alternatives

A previous attempt at this tried to reduce breakage by various tricks. This is still a possibility, but given that crater showed no regressions it seems unclear why we would want to introduce this complexity.

Disallowing `[foo; 0]` completely is also an option, but obviously this is more of a breaking change. I do not know how often this is actually used though.

r? `@oli-obk`
…vidtwco

Avoid double binding of subdiagnostics inside `#[derive(SessionDiagnostic)]`

r? `@davidtwco`
…ylan-DPC

Lifetime variance fixes for rustdoc

rust-lang#97287 migrates rustc to a `Ty` type that is invariant over its lifetime `'tcx`, so I need to fix a bunch of places that assume that `Ty<'a>` and `Ty<'b>` can be unified by shortening both to some common lifetime.

This is doable, since everything is already `'tcx`, so all this PR does is be a bit more explicit that elided lifetimes are actually `'tcx`.

Split out from rust-lang#97287 so the rustdoc team can review independently.
…, r=davidtwco

Parse expression after `else` as a condition if followed by `{`

Fixes rust-lang#49361.

Two things:
1. This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅
2. Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an `if`" case is the right one.
explain how to turn integers into fn ptrs

(with an intermediate raw ptr, not a direct transmute)
Direct int2ptr transmute, under the semantics I am imagining, will produce a ptr with "invalid" provenance that is invalid to deref or call. We cannot give it the same semantics as int2ptr casts since those do [something complicated](https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html).

To my great surprise, that is already what the example in the `transmute` docs does. :)  I still added a comment to say that that part is important, and I added a section explicitly talking about this to the `fn()` type docs.

With rust-lang/miri#2151, Miri will start complaining about direct int-to-fnptr transmutes (in the sense that it is UB to dereference the resulting pointer).
@rustbot rustbot added 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels May 24, 2022
@Dylan-DPC
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented May 24, 2022

📌 Commit dd60378 has been approved by Dylan-DPC

@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 May 24, 2022
@bors
Copy link
Contributor

bors commented May 24, 2022

⌛ Testing commit dd60378 with merge e71513c6d5a89d80bf4cc3a6117a8305cbaff755...

@rust-log-analyzer
Copy link
Collaborator

The job test-various failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [ui] src/test/ui/wrong-hashset-issue-42918.rs ... ok

failures:

---- [ui] src/test/ui/drop/repeat-drop.rs stdout ----
error: test run failed!
status: exit status: 101
status: exit status: 101
command: "/node-v15.14.0-linux-x64/bin/node" "/checkout/src/etc/wasm32-shim.js" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/repeat-drop/a.wasm"
stdout: none
--- stderr -------------------------------
RuntimeError: unreachable
    at __rust_start_panic (<anonymous>:wasm-function[70]:0x4482)
    at rust_panic (<anonymous>:wasm-function[68]:0x446e)
    at _ZN3std9panicking20rust_panic_with_hook17h438412e025c69b41E (<anonymous>:wasm-function[67]:0x443e)
    at _ZN3std9panicking11begin_panic28_$u7b$$u7b$closure$u7d$$u7d$17h3fa2c125e85564d1E (<anonymous>:wasm-function[2]:0x23a)
    at _ZN3std10sys_common9backtrace26__rust_end_short_backtrace17h175251abdd52dbe6E (<anonymous>:wasm-function[1]:0x1ff)
    at _ZN3std9panicking11begin_panic17hcf6af3f636240d9fE (<anonymous>:wasm-function[5]:0x2c4)
    at _ZN4core3ptr43drop_in_place$LT$repeat_drop..DropPanic$GT$17h4c782ed515570b9bE (<anonymous>:wasm-function[8]:0x313)
    at _ZN11repeat_drop4main17h9a2d0884828a3022E (<anonymous>:wasm-function[11]:0x3aa)
    at _ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17h1a420364536eafdfE (<anonymous>:wasm-function[3]:0x246)
    at _ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17h1a9ec836ff36213bE (<anonymous>:wasm-function[4]:0x268)
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=wasm32-unknown-unknown
    at _ZN3std2rt19lang_start_internal17h550cf429c961372bE (<anonymous>:wasm-function[49]:0x33ea)
    at main (<anonymous>:wasm-function[12]:0x3e1)
    at Object.<anonymous> (/checkout/src/etc/wasm32-shim.js:20:20)
    at Module._compile (node:internal/modules/cjs/loader:1092:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47



failures:

@bors
Copy link
Contributor

bors commented May 24, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 24, 2022
@JohnTitor
Copy link
Member

Failed by #95953

@JohnTitor JohnTitor closed this May 24, 2022
@Dylan-DPC Dylan-DPC deleted the rollup-jv8y7lm branch May 24, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup 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. 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.

9 participants