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 17 pull requests #57354

Merged
merged 40 commits into from
Jan 5, 2019
Merged

Rollup of 17 pull requests #57354

merged 40 commits into from
Jan 5, 2019

Conversation

kennytm
Copy link
Member

@kennytm kennytm commented Jan 5, 2019

Successful merges:

Failed merges:

r? @ghost

matthewjasper and others added 30 commits December 28, 2018 23:39
The types are no longer used with the change to stacked borrows for
validation.
There are three problems with the nolink-with-link-args test:

* The test fails when using MSVC. It's caused by the `linker-flavor=ld` flag which was added in rust-lang#46291.
* In its comment, this test tests that "link_args are indeed passed when nolink is specified", but the `nolink` attribute has been removed [a long time ago](rust-lang#12826).
* Pattern has a small typo.

At first I was going to completely remove this test, but there is [a closed pull request for that](rust-lang#21090).

So:

* rename the file as suggested in the closed PR
* adjust the comment
* fix typo in the pattern
* add `ignore-msvc`.
Add a new cmpxchg16b intrinsics for x86_64!
I noticed a duplicated "be" somewhere in the code. A search for it
manifested a couple more locations with the same problem. This change
removes one of the "be"s.
VaList::copy does not need to take a mutable reference. The va_copy
intrinsic takes a immutable reference.
Found with `git grep -P '\b([a-z]+)\s+\1\b'`
Trying to write a `const_fn` lint for Clippy. @oli-obk suggested
[here][link] to use the `is_min_const_fn` function from the
`qualify_min_const_fn` module. However, the module is currently private
and this commit makes it public.

I lack any historical knowledge of the development of the `const_fn`
feature, so I'm not sure if it was private on purpose or not. fwiw, all
modules are already public except `qualify_min_const_fn`.

[link]: rust-lang/rust-clippy#2440 (comment)
It's possible that `is_object_safe` is called on a trait that is ill-formed, and we shouldn't ICE unless there are no errors being raised. Using `delay_span_bug` solves this.

fixes rust-lang#56806
Fix rust-lang#56806 by using `delay_span_bug` in object safety layout sanity checks

It's possible that `is_object_safe` is called on a trait method that with an invalid receiver type. This caused an ICE in rust-lang#56806, because `receiver_is_dispatchable` returns `true` for `self: Box<dyn Trait>`, which causes one of the layout sanity checks in object_safety.rs to fail. Replacing `bug!` with `delay_span_bug` solves this.

The fact that `receiver_is_dispatchable` returns `true` here could be considered a bug. It passes the check that the method implements, though: `Box<dyn Trait>` implements `DispatchFromDyn<Box<dyn Trait>>` because `dyn Trait` implements `Unsize<dyn Trait>`. It would be good to hear what @eddyb and @nikomatsakis think.

Note that I only added a test for the case encountered in rust-lang#56806. I could not come up with a case that triggered an ICE from the other check, `bug!("receiver when Self = dyn Trait should be ScalarPair, found Scalar")`. There is no way, to my knowledge, that you can make `receiver_is_dispatchable` return true but still have a `Scalar` ABI when `Self = dyn Trait`.

One other case I encountered while debugging rust-lang#56806 was that if you have a type parameter `T` that implements `Deref<Target=Self>` and `DispatchFromDyn<T>`, and use it as a method receiver, it will cause an ICE during `is_object_safe` because `T` has no layout ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=d9b7497b3be0ca8382fa7d9497263214)):

```rust
trait Trait<T: Deref<Target=Self> + DispatchFromDyn<T>> {
    fn foo(self: T) -> dyn Trait<T>;
}
```

I don't intend to remove the ICE there because it is a pathological case, especially since there is no way to implement `DispatchFromDyn<T>` for `T` — the checks in typeck/coherence/builtin.rs do not allow that.

fixes rust-lang#56806
r? @varkor
…=alexcrichton

Rename and fix nolink-with-link-args test

There are three problems with the nolink-with-link-args test:

* The test fails when using MSVC. It's caused by the `linker-flavor=ld` flag which was added in rust-lang#46291.
* In its comment, this test tests that "link_args are indeed passed when nolink is specified", but the `nolink` attribute has been removed [a long time ago](rust-lang#12826).
* Pattern has a small typo.

At first I was going to completely remove this test, but there is [a closed pull request for that](rust-lang#21090).

So:

* rename the file as suggested in the closed PR
* adjust the comment
* fix typo in the pattern
* add `ignore-msvc`.

r? @alexcrichton
Fix backtraces for inlined functions on Windows

Fixes an regression introduced in rust-lang#50526

r? @alexcrichton
…=KodrAus

Fix broken links to second edition TRPL.

Fixes rust-lang#57104.

Remove `second-edition/` from TRPL hyperlinks.
src/jemalloc is gone, remove its mention from COPYRIGHT

The `src/jemalloc` submodule was removed in 61e8944 / rust-lang#55238.
…matsakis

Update the stdsimd submodule

Add a new cmpxchg16b intrinsics for x86_64 and some corrections for ARM/AArch64
kennytm added 10 commits January 5, 2019 23:56
Fix 'be be' constructs

I noticed a duplicated "be" somewhere in the code. A search for it
manifested a couple more locations with the same problem. This change
removes one of the "be"s.
VaList::copy should not require a mutable ref

`VaList::copy` does not need to take a mutable reference. The `va_copy`
intrinsic takes a immutable reference.
`const fn` is no longer coming soon (const keyword docs)

The `const` keyword [documentation](https://doc.rust-lang.org/std/keyword.const.html) mentions that `const fn`s are coming soon, but they have already been added.
Improve Box<T> -> Pin<Box<T>> conversion

I found the `From` trait conversion for this very hard to find, having a named function for it is much more discoverable. Also fixes rust-lang#56256 as I need that in the place I'm using this.

Has a placeholder tracking issue, will file an issue once I get feedback.
Fix repeated word typos

Inspired by rust-lang#57295 (I skipped 'be be' because of it) and my [PR in another repo
](cp-algorithms/cp-algorithms#389)
Not a stupid `sed`, I actually tried to fix case by case.
Doc rewording, use the same name `writer`

None
…r=GuillaumeGomez

rustdoc: force binary filename for compiled doctests

Fixes rust-lang#57317, needed for rust-lang/rust-by-example#1137

Right now, when building a doctest, rustdoc provides the compiler an output directory (a temp dir) but lets the compiler name the executable. If the doctest needs to be executed, it then tries to run a binary named `rust_out` from that directory. For the most part, this works fine. However, if the doctest sets its own crate name, the compiler uses that name for the output binary instead. This causes rustdoc to try to execute a nonexistent binary, causing the test to fail.

This PR changes the paths rustdoc gives to the compiler when building doctests to force the output *filename* instead of just the *directory*.
librustc_mir: Make qualify_min_const_fn module public

Trying to write a `const_fn` lint for Clippy. @oli-obk suggested
[here][link] to use the `is_min_const_fn` function from the
`qualify_min_const_fn` module. However, the module is currently private
and this commit makes it public.

I lack any historical knowledge of the development of the `const_fn`
feature, so I'm not sure if it was private on purpose or not. fwiw, all
modules are already public except `qualify_min_const_fn`.

r? @oli-obk

[link]: rust-lang/rust-clippy#2440 (comment)
…komatsakis

Calculate privacy access only via query

Initially converted to query in rust-lang@a9f6bab and then changed to respect dependencies rust-lang@8281e88.

I did this as an effort to prune `CrateAnalysis` from librustc_save_analysis, with the only thing remaining being the glob map (`name` is unused, existing `crate_name` is exposed in the compiler passes, instead).

Since calculating the glob map is opt-in, it'd be great if we could calculate that on-demand. However, it seems that it'd require converting resolution to queries, which I'm not sure how to do yet.

In an effort to get rid of `CrateAnalysis` altogether, could we try unconditionally calculating the glob_map in the resolver, thus completely removing `CrateAnalysis` struct, and doing a perf run?

r? @nikomatsakis

cc @petrochenkov do you have any idea how/if at all could we querify the resolver? I've stumbled upon a comment that's ~3? years old at the moment, so I'm guessing things might have changed and it actually may be feasible now. https://github.com/rust-lang/rust/blob/fe0c10019d7ee96909cc42cc265ef999a6b5dd70/src/librustc_driver/driver.rs#L589-L593
@kennytm
Copy link
Member Author

kennytm commented Jan 5, 2019

@bors r+ p=17

@bors
Copy link
Contributor

bors commented Jan 5, 2019

📌 Commit d9885c4 has been approved by kennytm

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

bors commented Jan 5, 2019

⌛ Testing commit d9885c4 with merge 68fe518...

bors added a commit that referenced this pull request Jan 5, 2019
Rollup of 17 pull requests

Successful merges:

 - #57219 (Remove some unused code)
 - #57229 (Fix #56806 by using `delay_span_bug` in object safety layout sanity checks)
 - #57233 (Rename and fix nolink-with-link-args test)
 - #57238 (Fix backtraces for inlined functions on Windows)
 - #57249 (Fix broken links to second edition TRPL.)
 - #57267 (src/jemalloc is gone, remove its mention from COPYRIGHT)
 - #57273 (Update the stdsimd submodule)
 - #57278 (Add Clippy to config.toml.example)
 - #57295 (Fix 'be be' constructs)
 - #57311 (VaList::copy should not require a mutable ref)
 - #57312 (`const fn` is no longer coming soon (const keyword docs))
 - #57313 (Improve Box<T> -> Pin<Box<T>> conversion)
 - #57314 (Fix repeated word typos)
 - #57326 (Doc rewording, use the same name `writer`)
 - #57338 (rustdoc: force binary filename for compiled doctests)
 - #57342 (librustc_mir: Make qualify_min_const_fn module public)
 - #57343 (Calculate privacy access only via query)

Failed merges:

 - #57340 (Use correct tracking issue for c_variadic)

r? @ghost
@bors
Copy link
Contributor

bors commented Jan 5, 2019

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing 68fe518 to master...

@bors bors merged commit d9885c4 into rust-lang:master Jan 5, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #57354!

Tested on commit 68fe518.
Direct link to PR: #57354

🎉 rust-by-example on windows: test-fail → test-pass (cc @steveklabnik @marioidival @projektir, @rust-lang/infra).
🎉 rust-by-example on linux: test-fail → test-pass (cc @steveklabnik @marioidival @projektir, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jan 5, 2019
Tested on commit rust-lang/rust@68fe518.
Direct link to PR: <rust-lang/rust#57354>

🎉 rust-by-example on windows: test-fail → test-pass (cc @steveklabnik @marioidival @projektir, @rust-lang/infra).
🎉 rust-by-example on linux: test-fail → test-pass (cc @steveklabnik @marioidival @projektir, @rust-lang/infra).
@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
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.

Receiver when Self = () should have a Scalar ABI, found ScalarPair ...