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

NLL: cast causes failure to promote to static #55385

Merged
merged 3 commits into from Oct 27, 2018

Conversation

@davidtwco
Member

davidtwco commented Oct 26, 2018

Fixes #55288. See commit messages for more details.

r? @oli-obk
cc @nikomatsakis
cc @pnkfelix
cc @RalfJung

Show resolved Hide resolved src/librustc/mir/visit.rs Outdated
PlaceContext::NonUse(NonUseContext::StorageDead) |
PlaceContext::NonUse(NonUseContext::AscribeUserTy) |
PlaceContext::NonUse(NonUseContext::Validate) => false,
}

This comment has been minimized.

@RalfJung

RalfJung Oct 26, 2018

Member

What is this used for? It's a new classification, isn't it?

This comment has been minimized.

@davidtwco

davidtwco Oct 26, 2018

Member

I moved this function from another pass that I stumbled upon while making changes, it was used in a handful of places and was being imported from that pass.

Show resolved Hide resolved src/librustc/mir/visit.rs Outdated
pub enum PlaceContext<'tcx> {
NonMutatingUse(NonMutatingUseContext),
MutatingUse(MutatingUseContext),
MaybeMutatingUse(MaybeMutatingUseContext<'tcx>),

This comment has been minimized.

@RalfJung

RalfJung Oct 26, 2018

Member

This is not nice... now we have a mixture of type-based classification and dynamic classification, not getting the benefits of either of them.

Given this, I don't think it makes any sense to separate the enum into 4.

This comment has been minimized.

@davidtwco

davidtwco Oct 26, 2018

Member

I've modified this to remove the fourth variant. It's still not ideal, because there are now three borrow variants, but I think it is cleaner than with four variants.

Show resolved Hide resolved src/librustc/mir/visit.rs Outdated
Show resolved Hide resolved src/librustc/mir/visit.rs Outdated
Show resolved Hide resolved src/librustc_codegen_llvm/mir/analyze.rs Outdated
Show resolved Hide resolved src/librustc_mir/transform/const_prop.rs Outdated
Show resolved Hide resolved src/librustc_mir/transform/simplify.rs
@rust-highfive

This comment was marked as resolved.

Collaborator

rust-highfive commented Oct 26, 2018

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:17548ce7:start=1540553630592160539,finish=1540553683561884755,duration=52969724216
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
---
[00:52:35] .................................................................................................... 2200/4952
[00:52:39] .................................................................................................... 2300/4952
[00:52:43] .................................................................................................... 2400/4952
[00:52:47] .................................................................................................... 2500/4952
[00:52:50] .....................................................iiiiiiiii...................................... 2600/4952
[00:52:57] ...ii............................................................................................... 2800/4952
[00:53:00] .................................................................................................... 2900/4952
[00:53:04] ..............................................................................................i..... 3000/4952
[00:53:06] .................................................................................................... 3100/4952
---
travis_time:start:test_mir-opt
Check compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:06:13] 
[01:06:13] running 48 tests
[01:06:19] ERROR 2018-10-26T12:41:13Z: compiletest::runtest: None
[01:06:33] F....................F..........................
[01:06:33] 
[01:06:33] ---- [mir-opt] mir-opt/copy_propagation.rs stdout ----
[01:06:33] ---- [mir-opt] mir-opt/copy_propagation.rs stdout ----
[01:06:33] thread '[mir-opt] mir-opt/copy_propagation.rs' panicked at 'Did not find expected line, error: ran out of mir dump to match against
[01:06:33] Expected Line: "     _0 = _1;"
[01:06:33] Test Name: rustc.test.CopyPropagation.after.mir
[01:06:33] ... (elided)
[01:06:33]  bb0: {
[01:06:33] ... (elided)
[01:06:33]      _0 = _1;
[01:06:33]      _0 = _1;
[01:06:33] ... (elided)
[01:06:33]      return;
[01:06:33]  }
[01:06:33] Actual:
[01:06:33] fn test(_1: u32) -> u32{
[01:06:33]     let mut _0: u32;
[01:06:33]     scope 1 {
[01:06:33]     scope 2 {
[01:06:33]         let _2: u32;
[01:06:33]     }
[01:06:33]     }
[01:06:33]     bb0: {                              
[01:06:33]         StorageLive(_2);
[01:06:33]         _2 = _1;
[01:06:33]         _0 = _2;
[01:06:33]         StorageDead(_2);
[01:06:33]         return;
[01:06:33]     bb1: {
[01:06:33]         resume;
[01:06:33]     }
[01:06:33] }', tools/compiletest/src/runtest.rs:2939:13
[01:06:33] }', tools/compiletest/src/runtest.rs:2939:13
[01:06:33] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:06:33] 
[01:06:33] ---- [mir-opt] mir-opt/inline-closure-borrows-arg.rs stdout ----
[01:06:33] thread '[mir-opt] mir-opt/inline-closure-borrows-arg.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[01:06:33] Current block: None
[01:06:33] Actual Line: "        _9 = move (_5.0: &i32);"
[01:06:33] Expected Line: "    _8 = move (_5.0: &i32);"
[01:06:33] Test Name: rustc.foo.Inline.after.mir
[01:06:33] ... (elided)
[01:06:33] ... (elided)
[01:06:33] bb0: {
[01:06:33] ... (elided)
[01:06:33] ... (elided)
[01:06:33]     _3 = [closure@NodeId(53)];
[01:06:33] ... (elided)
[01:06:33]     _4 = &_3;
[01:06:33] ... (elided)
[01:06:33]     _6 = &(*_2);
[01:06:33] ... (elided)
[01:06:33]     _7 = &(*_2);
[01:06:33]     _5 = (move _6, move _7);
[01:06:33]     _8 = move (_5.0: &i32);
[01:06:33]     _9 = move (_5.1: &i32);
[01:06:33] ... (elided)
[01:06:33]     _0 = (*_8);
[01:06:33] ... (elided)
[01:06:33]     return;
[01:06:33] }
[01:06:33] ... (elided)
[01:06:33] Actual:
[01:06:33] fn foo(_1: T, _2: &i32) -> i32{
[01:06:33]     let mut _0: i32;
[01:06:33]     scope 1 {
[01:06:33]         scope 3 {
[01:06:33]     }
[01:06:33]     scope 2 {
[01:06:33]     scope 2 {
[01:06:33]         let _3: [closure@NodeId(53)];
[01:06:33]     scope 4 {
[01:06:33]     }
[01:06:33]     scope 5 {
[01:06:33]         let _8: &i32;
[01:06:33]         let _8: &i32;
[01:06:33]     }
[01:06:33]     let mut _4: &[closure@NodeId(53)];
[01:06:33]     let mut _5: (&i32, &i32);
[01:06:33]     let mut _6: &i32;
[01:06:33]     let mut _7: &i32;
[01:06:33]     let mut _9: &i32;
[01:06:33]     let mut _10: &i32;
[01:06:33]     bb0: {                              
[01:06:33]         StorageLive(_3);
[01:06:33]         _3 = [closure@NodeId(53)];
[01:06:33]         StorageLive(_4);
[01:06:33]         _4 = &_3;
[01:06:33]         StorageLive(_5);
[01:06:33]         StorageLive(_6);
[01:06:33]         _6 = &(*_2);
[01:06:33]         StorageLive(_7);
[01:06:33]         _7 = &(*_2);
[01:06:33]         _5 = (move _6, move _7);
[01:06:33]         _9 = move (_5.0: &i32);
[01:06:33]         _10 = move (_5.1: &i32);
[01:06:33]         StorageLive(_8);
[01:06:33]         _8 = _9;
[01:06:33]         _0 = (*_8);
[01:06:33]         StorageDead(_8);
[01:06:33]         StorageDead(_5);
[01:06:33]         StorageDead(_7);
[01:06:33]         StorageDead(_6);
[01:06:33]         StorageDead(_4);
[01:06:33]         StorageDead(_3);
[01:06:33]         return;
[01:06:33] }', tools/compiletest/src/runtest.rs:2939:13
[01:06:33] 
[01:06:33] 
[01:06:33] failures:
---
[01:06:33] 
[01:06:33] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[01:06:33] 
[01:06:33] 
[01:06:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/mir-opt" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/mir-opt" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "mir-opt" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:06:33] 
[01:06:33] 
[01:06:33] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:06:33] Build completed unsuccessfully in 0:18:48
[01:06:33] Build completed unsuccessfully in 0:18:48
[01:06:33] Makefile:58: recipe for target 'check' failed
[01:06:33] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:03da2872
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@oli-obk

This comment has been minimized.

Contributor

oli-obk commented Oct 26, 2018

@bors r+ 🏎

@bors

This comment has been minimized.

Contributor

bors commented Oct 26, 2018

📌 Commit dcaf3fd has been approved by oli-obk

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 26, 2018

@bors p=1

This is an RC2 "nice to have"

@bors

This comment was marked as resolved.

Contributor

bors commented Oct 26, 2018

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout issue-55288 (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self issue-55288 --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging src/librustc_mir/transform/qualify_consts.rs
CONFLICT (content): Merge conflict in src/librustc_mir/transform/qualify_consts.rs
Auto-merging src/librustc_mir/transform/promote_consts.rs
Auto-merging src/librustc_mir/transform/const_prop.rs
Auto-merging src/librustc_mir/borrow_check/nll/type_check/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

@bors

This comment was marked as resolved.

Contributor

bors commented Oct 26, 2018

☔️ The latest upstream changes (presumably #55382) made this pull request unmergeable. Please resolve the merge conflicts.

@davidtwco

This comment has been minimized.

Member

davidtwco commented Oct 26, 2018

@bors r=oli-obk

@bors

This comment has been minimized.

Contributor

bors commented Oct 26, 2018

📌 Commit 6b33189 has been approved by oli-obk

@bors

This comment was marked as resolved.

Contributor

bors commented Oct 27, 2018

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout issue-55288 (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self issue-55288 --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging src/librustc_mir/borrow_check/nll/type_check/mod.rs
Auto-merging src/librustc_codegen_llvm/mir/analyze.rs
Auto-merging src/librustc/mir/visit.rs
CONFLICT (content): Merge conflict in src/librustc/mir/visit.rs
Automatic merge failed; fix conflicts and then commit the result.

@bors

This comment was marked as resolved.

Contributor

bors commented Oct 27, 2018

☔️ The latest upstream changes (presumably #55274) made this pull request unmergeable. Please resolve the merge conflicts.

davidtwco added some commits Oct 26, 2018

Add helpful logging statements.
This commit adds logging statements to `promote_consts` and
`qualify_consts` to make it easier to understand what it is doing.
Test for cast causing static promotion failure.
This commit adds a test that ensures that a cast in a static doesn't
stop const promotion within the static.
Refactor and add `PlaceContext::AscribeUserTy`.
This commit refactors `PlaceContext` to split it into four different
smaller enums based on if the context represents a mutating use,
non-mutating use, maybe-mutating use or a non-use (this is based on the
recommendation from @oli-obk on Zulip[1]).

This commit then introduces a `PlaceContext::AscribeUserTy` variant.
`StatementKind::AscribeUserTy` is now correctly mapped to
`PlaceContext::AscribeUserTy` instead of `PlaceContext::Validate`.
`PlaceContext::AscribeUserTy` can also now be correctly categorized as a
non-use which fixes an issue with constant promotion in statics after a
cast introduces a `AscribeUserTy` statement.

[1]: https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/subject/.2355288.20cast.20fails.20to.20promote.20to.20'static/near/136536949
@davidtwco

This comment has been minimized.

Member

davidtwco commented Oct 27, 2018

@bors r=oli-obk

@bors

This comment has been minimized.

Contributor

bors commented Oct 27, 2018

📌 Commit 6208bd8 has been approved by oli-obk

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Oct 27, 2018

@bors p=1

As mentioned above, this is nice to have for RC2, so we are trying to win the race to get it in before beta cutoff

@bors

This comment has been minimized.

Contributor

bors commented Oct 27, 2018

⌛️ Testing commit 6208bd8 with merge a47da3a...

bors added a commit that referenced this pull request Oct 27, 2018

Auto merge of #55385 - davidtwco:issue-55288, r=oli-obk
NLL: cast causes failure to promote to static

Fixes #55288. See commit messages for more details.

r? @oli-obk
cc @nikomatsakis
cc @pnkfelix
cc @RalfJung
@bors

This comment was marked as resolved.

Contributor

bors commented Oct 27, 2018

💥 Test timed out

@davidtwco

This comment has been minimized.

Member

davidtwco commented Oct 27, 2018

@bors retry

@oli-obk

This comment has been minimized.

Contributor

oli-obk commented Oct 27, 2018

@bors retry

@bors

This comment has been minimized.

Contributor

bors commented Oct 27, 2018

⌛️ Testing commit 6208bd8 with merge b3b8760...

bors added a commit that referenced this pull request Oct 27, 2018

Auto merge of #55385 - davidtwco:issue-55288, r=oli-obk
NLL: cast causes failure to promote to static

Fixes #55288. See commit messages for more details.

r? @oli-obk
cc @nikomatsakis
cc @pnkfelix
cc @RalfJung
@bors

This comment has been minimized.

Contributor

bors commented Oct 27, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing b3b8760 to master...

@bors bors merged commit 6208bd8 into rust-lang:master Oct 27, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@bors bors referenced this pull request Oct 27, 2018

Merged

Delayed CTFE backtraces #54487

@davidtwco davidtwco deleted the davidtwco:issue-55288 branch Oct 27, 2018

@pietroalbini

This comment has been minimized.

Member

pietroalbini commented Nov 19, 2018

Nominating for beta backport, since this fixes a beta regression.

cc @rust-lang/compiler

@alexcrichton alexcrichton added this to the Rust 2018 Release milestone Nov 19, 2018

bors added a commit that referenced this pull request Nov 19, 2018

Auto merge of #56077 - pietroalbini:beta-rollup, r=pietroalbini
[beta] Rollup backports

Merged and approved:

* #55385: NLL: cast causes failure to promote to static
* #56043: remove "approx env bounds" if we already know from trait
* #56003: do not propagate inferred bounds on trait objects if they involve `Self`
* #55852: Rewrite `...` as `..=` as a `MachineApplicable` 2018 idiom lint
* #55804: rustdoc: don't inline `pub use some_crate` unless directly asked to
* #56059: Increase `Duration` approximate equal threshold to 1us

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment