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 9 pull requests #70169

Closed
wants to merge 32 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

Mark-Simulacrum and others added 30 commits March 18, 2020 19:03
This simplifies the node manipulation, as we can (in later commits) always know
when traversing nodes that we are not in a shared root.
We no longer have a separate header because the shared root is gone; all code
can work solely with leafs now.
This makes ensure_root_is_owned return a reference to the (now guaranteed to
exist) root, allowing callers to operate on it without going through another
unwrap.

Unfortunately this is only rarely useful as it's frequently the case that both
the length and the root need to be accessed and field-level borrows in methods
don't yet exist.
Fixes rust-lang#55099

The minimized reproducer in issue rust-lang#55099 now compiles successfully.
This commit adds a regression test for it.
patch is required to avoid compiler errors by building src/libpanic_unwind/hermit.rs
As discussed on reddit, this commit addresses two issues with the
documentation of `mem::forget()`:

* The documentation of `mem::forget()` can confuse the reader because of the
  discrepancy between usage examples that show correct usage and the
  accompanying text which speaks of the possibility of double-free.  The
  text that says "if the panic occurs before `mem::forget` was called"
  refers to a variant of the second example that was never shown, modified
  to use `mem::forget` instead of `ManuallyDrop`.  Ideally the documentation
  should show both variants, so it's clear what it's talking about.

  Also, the double free could be fixed just by placing `mem::forget(v)`
  before the construction of `s`.  Since the lifetimes of `s` and `v`
  wouldn't overlap, there would be no point where panic could cause a double
  free.  This could be mentioned, and contrasted against the more robust fix
  of using `ManuallyDrop`.

* This sentence seems unjustified: "For some types, operations such as
  passing ownership (to a funcion like `mem::forget`) requires them to
  actually be fully owned right now [...]".  Unlike C++, Rust has no move
  constructors, its moves are (possibly elided) bitwise copies.  Even if you
  pass an invalid object to `mem::forget`, no harm should come to pass
  because `mem::forget` consumes the object and exists solely to prevent
  drop, so there no one left to observe the invalid state state.
…m::forget.

As pointed out by Ralf Jung, dangling references and boxes are
undefined behavior as per
https://doc.rust-lang.org/reference/behavior-considered-undefined.html
and the Miri checker.
Co-Authored-By: Ralf Jung <post@ralfj.de>
Co-Authored-By: lzutao <taolzu@gmail.com>
…sper

Prefetch some queries used by the metadata encoder

This brings the time for `metadata encoding and writing` for `syntex_syntax` from 1.338s to 0.997s with 6 threads in non-incremental debug mode.

r? @Mark-Simulacrum
Clarify the relationship between `forget()` and `ManuallyDrop`.

As discussed on reddit, this commit addresses two issues with the
documentation of `mem::forget()`:

* The documentation of `mem::forget()` can confuse the reader because of the
  discrepancy between usage examples that show correct usage and the
  accompanying text which speaks of the possibility of double-free.  The
  text that says "if the panic occurs before `mem::forget` was called"
  refers to a variant of the second example that was never shown, modified
  to use `mem::forget` instead of `ManuallyDrop`.  Ideally the documentation
  should show both variants, so it's clear what it's talking about.

  Also, the double free could be fixed just by placing `mem::forget(v)`
  before the construction of `s`.  Since the lifetimes of `s` and `v`
  wouldn't overlap, there would be no point where panic could cause a double
  free.  This could be mentioned, and contrasted against the more robust fix
  of using `ManuallyDrop`.

* This sentence seems unjustified: "For some types, operations such as
  passing ownership (to a funcion like `mem::forget`) requires them to
  actually be fully owned right now [...]".  Unlike C++, Rust has no move
  constructors, its moves are (possibly elided) bitwise copies.  Even if you
  pass an invalid object to `mem::forget`, no harm should come to pass
  because `mem::forget` consumes the object and exists solely to prevent
  drop, so there no one left to observe the invalid state state.
…cuviper

BTreeMap: remove shared root

This replaces the shared root with `Option`s in the BTreeMap code, and then slightly cleans up the node manipulation code taking advantage of the removal of the shared root. I expect that further simplification is possible, but wanted to get this posted for initial review.

Note that `BTreeMap::new()` continues to not allocate.

Benchmarks seem within the margin of error/unaffected, as expected for an entirely predictable branch.

```
 name                                 alloc-bench-a ns/iter  alloc-bench-b ns/iter  diff ns/iter  diff %  speedup
 btree::map::iter_mut_20              20                     21                                1   5.00%   x 0.95
 btree::set::clone_100                1,360                  1,439                            79   5.81%   x 0.95
 btree::set::clone_100_and_into_iter  1,319                  1,434                           115   8.72%   x 0.92
 btree::set::clone_10k                143,515                150,991                       7,476   5.21%   x 0.95
 btree::set::clone_10k_and_clear      142,792                152,916                      10,124   7.09%   x 0.93
 btree::set::clone_10k_and_into_iter  146,019                154,561                       8,542   5.85%   x 0.94
```
…ikomatsakis

Add regression test for TAIT lifetime inference (issue rust-lang#55099)

Fixes rust-lang#55099

The minimized reproducer in issue rust-lang#55099 now compiles successfully.
This commit adds a regression test for it.
…atsakis

remove unused imports

patch is required to avoid compiler errors by building src/libpanic_unwind/hermit.rs
doc: Add quote to .init_array

The current formatting is not good without quotes:
![without-quote](https://i.imgur.com/RkIm4cr.png)
@Dylan-DPC-zz
Copy link
Author

@bors r+ p=9 rollup=never

@bors
Copy link
Contributor

bors commented Mar 19, 2020

📌 Commit 44fdd5a 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 Mar 19, 2020
@bors
Copy link
Contributor

bors commented Mar 20, 2020

⌛ Testing commit 44fdd5a with merge 03eb808417bb303306b789f1790246d970e17bef...

@bors
Copy link
Contributor

bors commented Mar 20, 2020

💔 Test failed - checks-azure

@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 Mar 20, 2020
@JohnTitor
Copy link
Member

Left a comment, closing.

@JohnTitor JohnTitor closed this Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.