Re-implement leak check in terms of universes #58592

nikomatsakis · 2019-02-20T11:44:19Z

This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily correct, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056.

Fixes #58451
Fixes #46989
Fixes #57639

r? @aturon
cc @arielb1, @lqd

Temporary note: I've not finished running ./x.py test locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.

rust-highfive · 2019-02-20T11:59:11Z

The job x86_64-gnu-llvm-6.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:362df60e:start=1550663127255856788,finish=1550663129465543049,duration=2209686261
$ 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 IMAGE=x86_64-gnu-llvm-6.0
---

[00:04:10] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:11] tidy error: /checkout/src/librustc/infer/mod.rs:968: line longer than 100 chars
[00:04:12] some tidy checks failed
[00:04:12] 
[00:04:12] 
[00:04:12] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:12] 
[00:04:12] 
[00:04:12] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:12] Build completed unsuccessfully in 0:00:45
[00:04:12] Build completed unsuccessfully in 0:00:45
[00:04:12] make: *** [tidy] Error 1
[00:04:12] Makefile:68: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:2cec4ad8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Feb 20 11:49:53 UTC 2019
---
travis_time:end:15196653:start=1550663393681474415,finish=1550663393685718651,duration=4244236
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:19e3adfa
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1de0386f
travis_time:start:1de0386f
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:03da50bf
$ dmesg | grep -i kill

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)

aturon

I reviewed the new commits here and they LGTM, modulo a sanity check re: the role of snapshots.

aturon · 2019-02-20T17:41:05Z

src/librustc/infer/region_constraints/leak_check.rs

+        tcx: TyCtxt<'_, '_, 'tcx>,
+        overly_polymorphic: bool,
+        placeholder_map: &PlaceholderMap<'tcx>,
+        _snapshot: &CombinedSnapshot<'_, 'tcx>,


Just to confirm: this parameter is basically acting as a static check that we're in the scope of a snapshot?

Confirm, I should perhaps write that more explicitly as a comment.

nikomatsakis · 2019-02-20T19:47:20Z

@bors try

bors · 2019-02-20T19:47:33Z

⌛ Trying commit f156ac2 with merge c3b2220...

@aturon

[WIP] Re-implement leak check in terms of universes This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily **correct**, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056. Fixes #58451 Fixes #46989 Fixes #57639 r? @aturon cc @arielb1, @lqd ~~Temporary note: I've not finished running `./x.py test` locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.~~

Mark-Simulacrum · 2019-02-20T20:15:42Z

Nominating for compiler team approval for beta backport because it fixes several regressions.

bors · 2019-02-20T22:12:25Z

☀️ Test successful - checks-travis
State: approved= try=True

Mark-Simulacrum · 2019-02-20T22:14:29Z

@rust-timer build c3b2220

rust-timer · 2019-02-20T22:14:30Z

Success: Queued c3b2220 with parent f66e469, comparison URL.

Mark-Simulacrum · 2019-02-20T22:15:51Z

@craterbot run start=master#f66e4697ae286985ddefc53c3a047614568458bb end=try#c3b22200e6f5b70eb1f99ae6944d989ae17a458a mode=check-only p=5

craterbot · 2019-02-20T22:15:54Z

👌 Experiment pr-58592 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2019-02-20T22:15:56Z

🚧 Experiment pr-58592 is now running on agent aws-1.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

pnkfelix · 2019-02-20T23:12:18Z

hey @nikomatsakis do you know off-hand if this will also address #57464 ?

rust-timer · 2019-02-20T23:56:19Z

Finished benchmarking try commit c3b2220

nikomatsakis · 2019-02-21T11:29:29Z

@pnkfelix it does not appear to fix that test case

bors · 2019-02-21T21:35:18Z

⌛ Testing commit 33d3598 with merge 204212e528e170b563444616a989730d686b4513...

bors · 2019-02-21T21:35:28Z

⌛ Testing commit 33d3598 with merge 8a1d0de...

@aturon

Re-implement leak check in terms of universes This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily **correct**, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056. Fixes #58451 Fixes #46989 Fixes #57639 r? @aturon cc @arielb1, @lqd ~~Temporary note: I've not finished running `./x.py test` locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.~~

bors · 2019-02-22T00:23:52Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: aturon
Pushing 8a1d0de to master...

craterbot · 2019-02-22T05:04:28Z

🎉 Experiment pr-58592 is completed!
📊 2 regressed and 2 fixed (50551 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Centril · 2019-02-22T09:25:46Z

Regression tree:

google-testing1-cli-1.0.8+20181011:

[INFO] [stderr] Error response from daemon: endpoint with name confident_tesla already exists in network none

tokei-8.0.1

[INFO] [stderr] Error response from daemon: endpoint with name confident_tesla already exists in network none

Both spurious. We have no actual reported regressions 🎉

pnkfelix · 2019-02-22T12:29:41Z

We discussed this beta-nomination in the compiler team meeting yesterday, as part of the checkin with the WG-traits working group.

The executive summary is: We think the risk/reward here is in favor of approving the beta backport; but since this only landed in nightly so recently (notably, after the meeting where we were discussing it), we need to keep our eye out for problems.

Here is how @aturon summarized things (and a lot of meeting participants agreed with this synopsis):

i think we should:

land in nightly

land beta backport

start crater run on beta

basically get as much info about the impact as we can, ASAP. if any of these reveal problems, then we can delay the release

therefore, marking as beta-accepted.

[BETA] Universe leak check Backport of #58592 and #58056 to beta.

@pietroalbini

Beta rollups - #58649 - #58592 - #58574 - #58227 - #58056 - #57859 - #57710 A few of these are via #58641. r? @pietroalbini

The breaking change was reverted in rust-lang/rust#58592. This reverts commit 6fd0e82.

rust-highfive assigned aturon Feb 20, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 20, 2019

nikomatsakis mentioned this pull request Feb 20, 2019

Tracking issue for future-incompatibility lint coherence_leak_check #56105

Open

aturon approved these changes Feb 20, 2019

View reviewed changes

Mark-Simulacrum mentioned this pull request Feb 20, 2019

make generalization code create new variables in correct universe #58056

Merged

Mark-Simulacrum added I-nominated beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2019

nikomatsakis changed the title ~~[WIP] Re-implement leak check in terms of universes~~ Re-implement leak check in terms of universes Feb 20, 2019

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2019

nikomatsakis force-pushed the universe-leak-check branch from f156ac2 to b9df9b2 Compare February 21, 2019 11:29

lqd mentioned this pull request Feb 21, 2019

ICE related to lifetimes and traits #58451

Closed

nikomatsakis added 3 commits February 21, 2019 11:32

s/skol_/placeholder_/

3cd286b

reintroduce commit_if_ok call into higher_ranked_sub

6cb4ffb

reintroduce commit_if_ok calls to subtype_predicate

2cbe07b

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 22, 2019

bors merged commit 33d3598 into rust-lang:master Feb 22, 2019

bors mentioned this pull request Feb 22, 2019

[Step 1] Implement "small substs optimization" for substs of length 1 #58321

Merged

craterbot 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 Feb 22, 2019

pnkfelix added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed I-nominated labels Feb 22, 2019

nikomatsakis mentioned this pull request Feb 22, 2019

[BETA] Universe leak check #58641

Closed

bors added a commit that referenced this pull request Feb 22, 2019

Auto merge of #58641 - nikomatsakis:universe-leak-check-beta, r=<try>

799097d

[BETA] Universe leak check Backport of #58592 and #58056 to beta.

kennytm mentioned this pull request Feb 22, 2019

Spurious 30 minutes timeout on Travis CI #56112

Closed

Mark-Simulacrum mentioned this pull request Feb 22, 2019

Beta rollups #58656

Merged

pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 22, 2019

bors added a commit that referenced this pull request Feb 23, 2019

Auto merge of #58656 - Mark-Simulacrum:beta-rollups, r=pietroalbini

bb2d0ea

Beta rollups - #58649 - #58592 - #58574 - #58227 - #58056 - #57859 - #57710 A few of these are via #58641. r? @pietroalbini

dtolnay added a commit to dtolnay/rust-quiz that referenced this pull request Feb 23, 2019

Revert "Update question 5 for universe transition"

7432a2c

The breaking change was reverted in rust-lang/rust#58592. This reverts commit 6fd0e82.

jethrogb mentioned this pull request Feb 25, 2019

Function pointer docs may need updating #46989

Open

SimonSapin mentioned this pull request Feb 27, 2019

script-servo is broken again rust-lang/rustc-perf#354

Closed

lqd mentioned this pull request Feb 27, 2019

[NLL] Bad higher ranked subtype error #57374

Open

pnkfelix mentioned this pull request Mar 28, 2019

eventual goal: re-remove leak-check from compiler #59490

Open

nikomatsakis mentioned this pull request Sep 20, 2019

ICE on higher-trait bounds #60218

Closed

ecstatic-morse mentioned this pull request Jul 30, 2021

Check constraints arising from higher-rank trait bounds (HRTBs) rust-lang/polonius#172

Open

spastorino mentioned this pull request Aug 6, 2021

Consolidate HKTBs, leak-check and universes information around with what we have in the guide rust-lang/rustc-dev-guide#1184

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-implement leak check in terms of universes #58592

Re-implement leak check in terms of universes #58592

nikomatsakis commented Feb 20, 2019 •

edited

Loading

rust-highfive commented Feb 20, 2019

aturon left a comment

aturon Feb 20, 2019

nikomatsakis Feb 20, 2019

nikomatsakis commented Feb 20, 2019

bors commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

bors commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

rust-timer commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

craterbot commented Feb 20, 2019

craterbot commented Feb 20, 2019

pnkfelix commented Feb 20, 2019

rust-timer commented Feb 20, 2019

nikomatsakis commented Feb 21, 2019

bors commented Feb 21, 2019

bors commented Feb 21, 2019

bors commented Feb 22, 2019

craterbot commented Feb 22, 2019

Centril commented Feb 22, 2019

pnkfelix commented Feb 22, 2019

Re-implement leak check in terms of universes #58592

Re-implement leak check in terms of universes #58592

Conversation

nikomatsakis commented Feb 20, 2019 • edited Loading

rust-highfive commented Feb 20, 2019

aturon left a comment

Choose a reason for hiding this comment

aturon Feb 20, 2019

Choose a reason for hiding this comment

nikomatsakis Feb 20, 2019

Choose a reason for hiding this comment

nikomatsakis commented Feb 20, 2019

bors commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

bors commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

rust-timer commented Feb 20, 2019

Mark-Simulacrum commented Feb 20, 2019

craterbot commented Feb 20, 2019

craterbot commented Feb 20, 2019

pnkfelix commented Feb 20, 2019

rust-timer commented Feb 20, 2019

nikomatsakis commented Feb 21, 2019

bors commented Feb 21, 2019

bors commented Feb 21, 2019

bors commented Feb 22, 2019

craterbot commented Feb 22, 2019

Centril commented Feb 22, 2019

pnkfelix commented Feb 22, 2019

nikomatsakis commented Feb 20, 2019 •

edited

Loading