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

Closed
wants to merge 17 commits into from
Closed

Conversation

RalfJung
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

nikomatsakis and others added 17 commits April 16, 2020 11:03
UI tests are updated with additional error messages that were missing
before.
Add testcases for the `#[track_caller]` and `#[target_feature(..)]`
function attributes for errors that were not not caught before.
We don't need the `scc_dependency_order` vector, `all_sccs` is already
in dependency order.
Otherwise each update is run twice and errors are printed
This was introduced in the recent bump to 1.43 bootstrap cargo
…tthewjasper

extend NLL checker to understand `'empty` combined with universes

This PR extends the NLL region checker to understand `'empty` combined with universes. In particular, it means that the NLL region checker no longer considers `exists<R2> { forall<R1> { R1: R2 } }` to be provable. This is work towards rust-lang#59490, but we're not all the way there. One thing in particular it does not address is error messages.

The modifications to the NLL region inference code turned out to be simpler than expected. The main change is to require that if `R1: R2` then `universe(R1) <= universe(R2)`.

This constraint follows from the region lattice (shown below), because we assume then that `R2` is "at least" `empty(Universe(R2))`, and hence if `R1: R2` (i.e., `R1 >= R2` on the lattice) then `R1` must be in some universe that can name `'empty(Universe(R2))`, which requires that `Universe(R1) <= Universe(R2)`.

```
static ----------+-----...------+       (greatest)
|                |              |
early-bound and  |              |
free regions     |              |
|                |              |
scope regions    |              |
|                |              |
empty(root)   placeholder(U1)   |
|            /                  |
|           /         placeholder(Un)
empty(U1) --         /
|                   /
...                /
|                 /
empty(Un) --------                      (smallest)
```

I also made what turned out to be a somewhat unrelated change to add a special region to represent `'empty(U0)`, which we use (somewhat hackily) to indicate well-formedness checks in some parts of the compiler. This fixes rust-lang#68550.

I did some investigation into fixing the error message situation. That's a bit trickier: the existing "nice region error" code around placeholders relies on having better error tracing than NLL currently provides, so that it knows (e.g.) that the constraint arose from applying a trait impl and things like that. I feel like I was hoping *not* to do such fine-grained tracing in NLL, and it seems like we...largely...got away with that. I'm not sure yet if we'll have to add more tracing information or if there is some sort of alternative.

It's worth pointing out though that I've not kind of shifted my opinion on whose job it should be to enforce lifetimes: I tend to think we ought to be moving back towards *something like* the leak-check (just not the one we *had*). If we took that approach, it would actually resolve this aspect of the error message problem, because we would be resolving 'higher-ranked errors' in the trait solver itself, and hence we wouldn't have to thread as much causal information back to the region checker. I think it would also help us with removing the leak check while not breaking some of the existing crates out there.

Regardless, I think it's worth landing this change, because it was relatively simple and it aligns the set of programs that NLL accepts with those that are accepted by the main region checker, and hence should at least *help* us in migration (though I guess we still also have to resolve the existing crates that rely on leak check for coherence).

r? @matthewjasper
…vink

rustc: fix check_attr() for methods, closures and foreign functions

This fixes an issue that previously turned up for methods in rust-lang#69274, but also exists for closures and foreign function: `check_attr` does not call `codegen_fn_attrs()` for these types when it should, meaning that incorrectly used function attributes are not diagnosed without codegen.

The issue affects our UI tests, as they run with `--emit=metadata` by default, but as it turns out, this is not the only case: Function attributes are not checked on any dead code without this fix!

This makes the fix a **breaking change**. The following very silly Rust programs compiles fine on stable Rust when it should not, which is fixed by this PR.
```rust
fn main() {
    #[target_feature(enable = "sse2")]
    || {};
}
```

I assume any real-world program which may trigger this issue would at least emit a dead code warning, but of course that is no guarantee that such code does not exist...

Fixes rust-lang#70307
Suggest deref when coercing `ty::Ref` to `ty::RawPtr`

Fixes rust-lang#32122

Currently we do autoderef when casting `ty::Ref` ->`ty::Ref`, but we don't autoderef when casting `ty::Ref` -> `ty::RawPtr`. This PR make the compiler suggests deref when coercing `ty::Ref` to `ty::RawPtr`
…, r=Mark-Simulacrum

Detect git version before attempting to use --progress

Otherwise each update is run twice and errors are printed

I've tested this with:
git version 2.8.2.windows.1 (Windows)
git version 2.26.2.266.ge870325ee8 (Linux built from source)
git version 2.17.1 (Linux)
git version 2.21.1 (Apple Git-122.3) (MacOS)

I've tested with Python 2.7 (Windows, Linux, MacOS), 3.6 (Linux), and 3.7 (MacOS)
…iaskrgr

Handle build completion message from Cargo

This was introduced in the recent bump to 1.44 bootstrap cargo

Fixes rust-lang#71561.
@RalfJung RalfJung added the rollup A PR which is a rollup label Apr 30, 2020
@RalfJung
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Apr 30, 2020

📌 Commit 91428ab has been approved by RalfJung

@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 Apr 30, 2020
@bors
Copy link
Contributor

bors commented Apr 30, 2020

⌛ Testing commit 91428ab with merge 722230e7c669a9aec95f97a33a258eada38683cb...

@bors
Copy link
Contributor

bors commented Apr 30, 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 Apr 30, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Apr 30, 2020

That's weird, there's not even an error message?

[TIMING] ToolBuild { compiler: Compiler { stage: 0, host: "x86_64-apple-darwin" }, target: "x86_64-apple-darwin", tool: "unstable-book-gen", path: "src/tools/unstable-book-gen", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [] } -- 89.248


command did not execute successfully: "/Users/runner/runners/2.166.4/work/1/s/build/x86_64-apple-darwin/stage0-tools-bin/unstable-book-gen" "/Users/runner/runners/2.166.4/work/1/s/src" "/Users/runner/runners/2.166.4/work/1/s/build/x86_64-apple-darwin/md-doc/unstable-book"
expected success, got: signal: 9


failed to run: /Users/runner/runners/2.166.4/work/1/s/build/bootstrap/debug/bootstrap dist
Build completed unsuccessfully in 0:01:32

Cc @rust-lang/infra

Giving up on this rollup then.

@RalfJung RalfJung closed this Apr 30, 2020
@Dylan-DPC-zz
Copy link

created a new rollup

@Dylan-DPC-zz
Copy link

unstable book was broken yest i think, so will check if the problem persists in the new rollup

@RalfJung RalfJung deleted the rollup-trho3le branch May 12, 2020 07:11
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants