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 #61044

Merged
merged 17 commits into from
May 23, 2019
Merged

Rollup of 8 pull requests #61044

merged 17 commits into from
May 23, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented May 22, 2019

Successful merges:

Failed merges:

r? @ghost

Aaron1011 and others added 17 commits May 13, 2019 01:52
Fixes rust-lang#60726

Previous, AutoTraitFinder would only try to project predicates when the
predicate type contained an inference variable. When finding auto
traits, we only project to try to unify inference variables - we don't
otherwise learn any new information about the required bounds.

However, this lead to failing to properly generate a negative auto trait
impl (indicating that a type never implements a certain auto trait) in
the following unusual scenario:

In almost all cases, a type has an (implicit) negative impl of an auto
trait due some other type having an explicit *negative* impl of that
auto trait. For example:

struct MyType<T> {
    field: *const T
}

has an implicit 'impl<T> !Send for MyType<T>', due to the explicit
negative impl (in libcore) 'impl<T: ?Sized> !Send for *const T'.

However, as exposed by the 'abi_stable' crate, this isn't always the
case. This minimzed example shows how a type can never implement
'Send', due to a projection error:

```
pub struct True;
pub struct False;

pub trait MyTrait {
    type Project;
}

pub struct MyStruct<T> {
    field: T
}

impl MyTrait for u8 {
    type Project = False;
}

unsafe impl<T> Send for MyStruct<T>
    where T: MyTrait<Project=True> {}

pub struct Wrapper {
    inner: MyStruct<u8>
}
```

In this example, `<u8 as MyTrait>::Project == True'
must hold for 'MyStruct<u8>: Send' to hold.
However, '<u8 as MyTrait>::Project == False' holds instead

To properly account for this unusual case, we need to call
'poly_project_and_unify' on *all* predicates, not just those with
inference variables. This ensures that we catch the projection error
that occurs above, and don't incorrectly determine that 'Wrapper: Send'
holds.
…ntation is FFI safe

This allows types like Option<NonZeroU8> to be used in FFI without triggering the improper_ctypes lint. This works by changing the is_repr_nullable_ptr function to consider an enum E to be FFI-safe if:

- E has no explicit #[repr(...)].
- It only has two variants.
- One of those variants is empty (meaning it has no fields).
- The other variant has only one field.
- That field is one of the following:
  - &T
  - &mut T
  - extern "C" fn
  - core::num::NonZero*
  - core::ptr::NonNull<T>
  - #[repr(transparent)] struct wrapper around one of the types in this list.
- The size of E and its field are both known and are both the same size (implying E is participating in the nonnull optimization).
Allow null-pointer-optimized enums in FFI if their underlying representation is FFI safe

I'm not sure if this requires an RFC. I attempted to start [a discussion on internals.rust-lang.org](https://internals.rust-lang.org/t/options-ffi-safety-and-guarantees-for-abi-compatibility-with-nonnull-optimizations/9784) and when no one really objected I figured I'd go ahead and try implementing this.

This allows types like `Option<NonZeroU8>` to be used in FFI without triggering the `improper_ctypes` lint. This works by changing the `is_repr_nullable_ptr` function to consider an enum `E` to be FFI-safe if:

- `E` has no explicit `#[repr(...)]`.
- It only has two variants.
- One of those variants is empty (meaning it has no fields).
- The other variant has only one field.
- That field is one of the following:
  - `&T`
  - `&mut T`
  - `extern "C" fn`
  - `core::num::NonZero*`
  - `core::ptr::NonNull<T>`
  - `#[repr(transparent)] struct` wrapper around one of the types in this list.
- The size of `E` and its field are both known and are both the same size (implying `E` is participating in the nonnull optimization).

This logic seems consistent with [the Rust nomicon](https://doc.rust-lang.org/nomicon/repr-rust.html).
…r=eddyb

Always try to project predicates when finding auto traits in rustdoc

Fixes rust-lang#60726

Previous, AutoTraitFinder would only try to project predicates when the
predicate type contained an inference variable. When finding auto
traits, we only project to try to unify inference variables - we don't
otherwise learn any new information about the required bounds.

However, this lead to failing to properly generate a negative auto trait
impl (indicating that a type never implements a certain auto trait) in
the following unusual scenario:

In almost all cases, a type has an (implicit) negative impl of an auto
trait due some other type having an explicit *negative* impl of that
auto trait. For example:

struct MyType<T> {
    field: *const T
}

has an implicit 'impl<T> !Send for MyType<T>', due to the explicit
negative impl (in libcore) 'impl<T: ?Sized> !Send for *const T'.

However, as exposed by the 'abi_stable' crate, this isn't always the
case. This minimzed example shows how a type can never implement
'Send', due to a projection error:

```
pub struct True;
pub struct False;

pub trait MyTrait {
    type Project;
}

pub struct MyStruct<T> {
    field: T
}

impl MyTrait for u8 {
    type Project = False;
}

unsafe impl<T> Send for MyStruct<T>
    where T: MyTrait<Project=True> {}

pub struct Wrapper {
    inner: MyStruct<u8>
}
```

In this example, `<u8 as MyTrait>::Project == True'
must hold for 'MyStruct<u8>: Send' to hold.
However, '<u8 as MyTrait>::Project == False' holds instead

To properly account for this unusual case, we need to call
'poly_project_and_unify' on *all* predicates, not just those with
inference variables. This ensures that we catch the projection error
that occurs above, and don't incorrectly determine that 'Wrapper: Send'
holds.
Add FAQ for NLL migration

r? @pnkfelix

cc @oli-obk @davidtwco @Centril Since you've provided feedback on the warning wording before.
…ts, r=oli-obk

Migrate from recursion to iterate on qualify consts visitor impl

r? @oli-obk
…excrichton

Simplify RefCell minimum_spanning_tree example

This simplifies the implementation of the `minimum_spanning_tree` example of `RefCell` in the `cell` module-level docs, avoiding an unnecessary recursive call. This also eliminates the need for a block to contain the scope of the borrow in this example. But since that use of a block served an important didactic purpose, we make up for this by instead introducing a block in the initial, simpler example of `RefCell`, where the point will hopefully be conveyed to the reader more easily.
…oli-obk

Make maybe_codegen_consume_direct iterate instead of doing recursion

r? @oli-obk
…lwoerister

rustc_metadata: parametrize schema::CrateRoot by 'tcx and rip out old unused incremental infra.

These are the first two commits of rust-lang#59953, already reviewed and approved by @michaelwoerister.

r? @michaelwoerister
Update clippy submodule

r? @Manishearth

If anyone is wondering where the odd old commits are coming from, we merged all beta backport commits and so into master in order to make sure we don't need to keep those branches around.
@Centril
Copy link
Contributor Author

Centril commented May 22, 2019

@bors r+ p=8 rollup=never

@bors
Copy link
Contributor

bors commented May 22, 2019

📌 Commit 6806517 has been approved by Centril

@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 22, 2019
@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Testing commit 6806517 with merge 11a8bf5312f1ba301069ff68d6529b7ed30f0a84...

@pietroalbini
Copy link
Member

@bors retry

Yielding priority to the beta.

@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Testing commit 6806517 with merge 11f01bf...

bors added a commit that referenced this pull request May 22, 2019
Rollup of 8 pull requests

Successful merges:

 - #60300 (Allow null-pointer-optimized enums in FFI if their underlying representation is FFI safe)
 - #60773 (Always try to project predicates when finding auto traits in rustdoc)
 - #60809 (Add FAQ for NLL migration)
 - #61023 (Migrate from recursion to iterate on qualify consts visitor impl)
 - #61029 (Simplify RefCell minimum_spanning_tree example)
 - #61030 (Make maybe_codegen_consume_direct iterate instead of doing recursion)
 - #61034 (rustc_metadata: parametrize schema::CrateRoot by 'tcx and rip out old unused incremental infra.)
 - #61037 (Update clippy submodule)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented May 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Centril
Pushing 11f01bf to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2019
@bors bors merged commit 6806517 into rust-lang:master May 23, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #61044!

Tested on commit 11f01bf.
Direct link to PR: #61044

🎉 clippy-driver on windows: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
🎉 clippy-driver on linux: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
🎉 rls on windows: build-fail → test-pass (cc @Xanewok, @rust-lang/infra).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 23, 2019
Tested on commit rust-lang/rust@11f01bf.
Direct link to PR: <rust-lang/rust#61044>

🎉 clippy-driver on windows: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
🎉 clippy-driver on linux: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
🎉 rls on windows: build-fail → test-pass (cc @Xanewok, @rust-lang/infra).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok, @rust-lang/infra).
@Xanewok
Copy link
Member

Xanewok commented May 23, 2019

@Centril are submodule updates subject to rollups?

@Centril
Copy link
Contributor Author

Centril commented May 23, 2019

@Xanewok depends on the submodule =) books and clippy are pretty safe. LLVM ==> absolutely not.

@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.