handle outlives predicates in trait evaluation #54624

arielb1 · 2018-09-27T21:55:03Z

This handles higher-ranked outlives predicates in trait evaluation the same way they are handled in projection.

Fixes #54302. I think this is a more correct fix than #54401 because it fixes the root case in evaluation instead of making evaluation used in less cases. However, we might want to go to a direction closer to @nikomatsakis's solution with Chalk.

r? @nikomatsakis

nikomatsakis

I agree this is good. I'm happy to land this in place of my PR. Left a few nits/thoughts, though.

Also, do you still think we need to keep that !data.has_late_bound_regions() check, though? If so, why?

nikomatsakis · 2018-09-28T19:06:48Z

src/librustc/traits/select.rs

+                if r_a == r_b {
+                    // for<'a> 'a: 'a. OK
+                    Ok(EvaluatedToOk)
+                } else if r_a.is_late_bound() || r_b.is_late_bound() {


Annoyingly, the current region solver has one extra case to consider: the case where r_a is 'static. In that case, know it outlives any r_b. But we support this case inconsistently owing to the leak-check. Not sure how relevant that is here.

In particular, even within the leak check, we don't generate a constraint like 'a <= 'static, and hence the leak-check doesn't fail:

rust/src/librustc/infer/region_constraints/mod.rs

Lines 731 to 733 in 80e6e3e

(_, &ReStatic) => {

// all regions are subregions of static, so we can ignore this

}

The caching code that is going wrong — however — only cares about predicate_must_hold, so it's ok to falsely fail from time to time. Still, we might want this to be is_late_bound() && is_late_bound()?

You mentioned coherence, though...

This is trait inference, so I don't think r_a can ever end up being literally 'static (as opposed to being some inference variable that happens to be equatable to 'static).

Actually, I think the only way something like this might happen if someone writes a literal where 'static: 'a bound in their trait. In this case, evaluation would return an error, but fulfillment would be ok. I'm not sure this inconsistency

Currently I did the EvaluatedToAmbig thing in intercrate mode, so this won't affect coherence.

arielb1 · 2018-09-28T21:23:43Z

Eventually I decided on adding support for the literal-ReStatic case, to not regress on handling where 'static: 'a where-clauses.

Fixes rust-lang#54302.

There is no reason not to do it.

This makes evaluation more consistent with fulfillment.

nikomatsakis · 2018-10-02T14:28:19Z

@bors r+

bors · 2018-10-02T14:28:20Z

📌 Commit 1069c0e has been approved by nikomatsakis

bors · 2018-10-03T04:46:21Z

⌛ Testing commit 1069c0e with merge 33a3327eb9e324cb13bca0eb2ffd9e23262c82fe...

bors · 2018-10-03T06:36:05Z

💔 Test failed - status-travis

rust-highfive · 2018-10-03T06:36:07Z

The job dist-x86_64-linux-alt 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:02f3f53c:start=1538548498697509639,finish=1538548498705474560,duration=7964921
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2825a210
$ 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:20ecfef3
travis_time:start:20ecfef3
$ 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:010e9740
$ 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)

kennytm · 2018-10-03T14:56:02Z

@bors retry travis-ci/travis-ci#9696

@nikomatsakis

…tsakis handle outlives predicates in trait evaluation This handles higher-ranked outlives predicates in trait evaluation the same way they are handled in projection. Fixes rust-lang#54302. I think this is a more correct fix than rust-lang#54401 because it fixes the root case in evaluation instead of making evaluation used in less cases. However, we might want to go to a direction closer to @nikomatsakis's solution with Chalk. r? @nikomatsakis

bors · 2018-10-04T03:08:29Z

⌛ Testing commit 1069c0e with merge c67ea54...

@nikomatsakis

handle outlives predicates in trait evaluation This handles higher-ranked outlives predicates in trait evaluation the same way they are handled in projection. Fixes #54302. I think this is a more correct fix than #54401 because it fixes the root case in evaluation instead of making evaluation used in less cases. However, we might want to go to a direction closer to @nikomatsakis's solution with Chalk. r? @nikomatsakis

bors · 2018-10-04T05:33:59Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing c67ea54 to master...

nnethercote · 2018-10-25T04:43:56Z

This hurt perf by ~1% for several benchmarks:
https://perf.rust-lang.org/compare.html?start=d0787284da6e25a018f2f94fcd3779d33d6150fc&end=c67ea54d4466f1c082f72b194c100bb4954449e6&stat=instructions:u

@arielb1: any thoughts?

rust-highfive assigned nikomatsakis Sep 27, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2018

arielb1 force-pushed the evaluate-outlives branch from 8cc25ce to 546768e Compare September 27, 2018 21:56

nikomatsakis requested changes Sep 28, 2018

View reviewed changes

arielb1 added 3 commits October 1, 2018 22:44

handle outlives predicates in trait evaluation

53a4b39

Fixes rust-lang#54302.

enable using the evaluation cache on predicates with LBRs

9d44e9e

There is no reason not to do it.

add a special case for literal 'static: 'a where-clauses

1069c0e

This makes evaluation more consistent with fulfillment.

arielb1 force-pushed the evaluate-outlives branch from fbb6b16 to 1069c0e Compare October 1, 2018 19:44

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2018

nikomatsakis mentioned this pull request Oct 2, 2018

make evaluation track whether outlives relationships mattered #54401

Closed

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 Oct 3, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2018

pietroalbini mentioned this pull request Oct 3, 2018

Rollup of 10 pull requests #54796

Closed

bors merged commit 1069c0e into rust-lang:master Oct 4, 2018

bors mentioned this pull request Oct 4, 2018

adopt "placeholders" to represent universally quantified regions #54649

Merged

pnkfelix mentioned this pull request Nov 9, 2018

apint fails to compile on beta #55806

Closed

spastorino mentioned this pull request Jun 10, 2020

Regression: Not well-formed HRTB doesn't compile #55498

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

handle outlives predicates in trait evaluation #54624

handle outlives predicates in trait evaluation #54624

arielb1 commented Sep 27, 2018

nikomatsakis left a comment

nikomatsakis Sep 28, 2018

nikomatsakis Sep 28, 2018

nikomatsakis Sep 28, 2018

arielb1 Sep 28, 2018

arielb1 Sep 28, 2018 •

edited

Loading

arielb1 commented Sep 28, 2018

nikomatsakis commented Oct 2, 2018

bors commented Oct 2, 2018

bors commented Oct 3, 2018

bors commented Oct 3, 2018

rust-highfive commented Oct 3, 2018

kennytm commented Oct 3, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018

nnethercote commented Oct 25, 2018

	(_, &ReStatic) => {
	// all regions are subregions of static, so we can ignore this
	}

handle outlives predicates in trait evaluation #54624

handle outlives predicates in trait evaluation #54624

Conversation

arielb1 commented Sep 27, 2018

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Sep 28, 2018

Choose a reason for hiding this comment

nikomatsakis Sep 28, 2018

Choose a reason for hiding this comment

nikomatsakis Sep 28, 2018

Choose a reason for hiding this comment

arielb1 Sep 28, 2018

Choose a reason for hiding this comment

arielb1 Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

arielb1 commented Sep 28, 2018

nikomatsakis commented Oct 2, 2018

bors commented Oct 2, 2018

bors commented Oct 3, 2018

bors commented Oct 3, 2018

rust-highfive commented Oct 3, 2018

kennytm commented Oct 3, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018

nnethercote commented Oct 25, 2018

arielb1 Sep 28, 2018 •

edited

Loading