adopt "placeholders" to represent universally quantified regions #54649

nikomatsakis · 2018-09-28T19:02:47Z

This does a few preliminary refactorings that lay some groundwork for moving towards universe integration. Two things, primarily:

Rename from "skolemized" to "placeholder"
When instantiating for<'a, 'b, 'c>, just create one universe for all 3 regions, and distinguish them from one another using the BoundRegion.
- This is more accurate, and I think that in general we'll be moving towards a model of separating "binder" (universe, debruijn index) from "index within binder" in a number of places.
- In principle, it feels the current setup of making lots of universes could lead to us doing the wrong thing, but I've actually not been able to come up with an example where this is so.

r? @scalexm
cc @arielb1

nikomatsakis · 2018-09-28T19:09:54Z

I'm mildly rethinking the question of whether to have multiple placeholders per universe. It annoys me that I can't think of an example where it would matter, though I thought I had one. Otherwise it seems just a hair more complex to manage for little benefit?

nikomatsakis · 2018-09-28T19:10:13Z

I feel great about the term placeholder though! 👍

bors · 2018-09-29T06:25:43Z

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

scalexm · 2018-10-01T13:14:29Z

src/librustc/infer/higher_ranked/README.md

-2. Replace all bound regions in the supertype with skolemized
-   equivalents. A "skolemized" region is just a new fresh region
+2. Replace all bound regions in the supertype with placeholder
+   equivalents. A "placeholder" region is just a new fresh region


There are still some occurences of "skolemization" and "skolemize" used as a verb :p

scalexm · 2018-10-01T13:18:43Z

src/librustc/infer/higher_ranked/mod.rs

+        snapshot: &CombinedSnapshot<'a, 'tcx>,
+    ) {
+        debug!("pop_placeholders({:?})", placeholder_map);
+        let skol_regions: FxHashSet<_> = placeholder_map.values().cloned().collect();


skol_regions -> placeholders_regions

scalexm · 2018-10-01T13:18:58Z

src/librustc/infer/higher_ranked/mod.rs

        self.borrow_region_constraints()
-            .pop_skolemized(self.universe(), &skol_regions, &snapshot.region_constraints_snapshot);
+            .pop_placeholders(
+                &skol_regions,


scalexm · 2018-10-01T13:20:42Z

src/librustc/infer/region_constraints/mod.rs

-/// which can reach a skolemized region, or both. We call such regions
+/// When working with placeholder regions, we often wish to find all of
+/// the regions that are either reachable from a placeholder region, or
+/// which can reach a placeholder region, or both. We call such regions
 /// *tained* regions.  This struct allows you to decide what set of


pre-existing: tained -> tainted

scalexm · 2018-10-01T13:23:18Z

src/librustc/traits/project.rs

@@ -206,15 +206,15 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(

    let infcx = selcx.infcx();
    infcx.commit_if_ok(|snapshot| {
-        let (skol_predicate, skol_map) =
-            infcx.skolemize_late_bound_regions(&obligation.predicate);
+        let (skol_predicate, placeholder_map) =


Maybe find a better name for skol_predicate?
(multiple occurences in other files)

nikomatsakis · 2018-10-03T20:10:28Z

@bors r=scalexm

bors · 2018-10-03T20:10:29Z

📌 Commit 575ff54993c53359504cce139eb56e714e0712fb has been approved by scalexm

bors · 2018-10-04T05:39:37Z

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

Avoids a linear walk over the regions at the end.

nikomatsakis · 2018-10-04T16:40:03Z

@bors r=scalexm

bors · 2018-10-04T16:40:04Z

📌 Commit 85b9956 has been approved by scalexm

bors · 2018-10-04T20:29:04Z

⌛ Testing commit 85b9956 with merge 8c4ad4e...

@scalexm

adopt "placeholders" to represent universally quantified regions This does a few preliminary refactorings that lay some groundwork for moving towards universe integration. Two things, primarily: - Rename from "skolemized" to "placeholder" - When instantiating `for<'a, 'b, 'c>`, just create one universe for all 3 regions, and distinguish them from one another using the `BoundRegion`. - This is more accurate, and I think that in general we'll be moving towards a model of separating "binder" (universe, debruijn index) from "index within binder" in a number of places. - In principle, it feels the current setup of making lots of universes could lead to us doing the wrong thing, but I've actually not been able to come up with an example where this is so. r? @scalexm cc @arielb1

bors · 2018-10-04T23:17:26Z

☀️ Test successful - status-appveyor, status-travis
Approved by: scalexm
Pushing 8c4ad4e to master...

rust-highfive assigned scalexm Sep 28, 2018

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

scalexm reviewed Oct 1, 2018

View reviewed changes

nikomatsakis force-pushed the universes-refactor-1 branch from 591c2ac to 575ff54 Compare October 3, 2018 18:00

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

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 4, 2018

nikomatsakis force-pushed the universes-refactor-1 branch from 575ff54 to fe04fe2 Compare October 4, 2018 14:13

nikomatsakis added 10 commits October 4, 2018 11:02

select.rs: rustfmt

a96510d

rename skolemized to placeholder

4cd4eae

reorder the placeholders and inference variables in hr::sub

f8dd084

create just one subuniverse per binder -- no need for more, really

4c2fc33

extend NLL universe code to have >1 placeholder within one universe

0887456

build up the placeholder indices as we go

cdb95b0

Avoids a linear walk over the regions at the end.

remove all occurences of skolemize

21aaaac

remove occurences of skolemization

068a6a2

fix typo: s/tained/tainted/

b99a8d7

s/skol_predicate/placeholder_predicate

85b9956

nikomatsakis force-pushed the universes-refactor-1 branch from fe04fe2 to 85b9956 Compare October 4, 2018 15:02

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 4, 2018

bors mentioned this pull request Oct 4, 2018

[NLL] Improve closure region bound errors #54798

Merged

bors merged commit 85b9956 into rust-lang:master Oct 4, 2018

bors mentioned this pull request Oct 4, 2018

Cleanup rustc/ty part 1 #54717

Merged

csmoe mentioned this pull request Oct 19, 2018

Clean up skolemization with placeholder rust-lang/rustc-dev-guide#211

Closed

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

adopt "placeholders" to represent universally quantified regions #54649

adopt "placeholders" to represent universally quantified regions #54649

nikomatsakis commented Sep 28, 2018

nikomatsakis commented Sep 28, 2018

nikomatsakis commented Sep 28, 2018

bors commented Sep 29, 2018

scalexm Oct 1, 2018

scalexm Oct 1, 2018

scalexm Oct 1, 2018

scalexm Oct 1, 2018

scalexm Oct 1, 2018

nikomatsakis commented Oct 3, 2018

bors commented Oct 3, 2018

bors commented Oct 4, 2018

nikomatsakis commented Oct 4, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018

adopt "placeholders" to represent universally quantified regions #54649

adopt "placeholders" to represent universally quantified regions #54649

Conversation

nikomatsakis commented Sep 28, 2018

nikomatsakis commented Sep 28, 2018

nikomatsakis commented Sep 28, 2018

bors commented Sep 29, 2018

scalexm Oct 1, 2018

Choose a reason for hiding this comment

scalexm Oct 1, 2018

Choose a reason for hiding this comment

scalexm Oct 1, 2018

Choose a reason for hiding this comment

scalexm Oct 1, 2018

Choose a reason for hiding this comment

scalexm Oct 1, 2018

Choose a reason for hiding this comment

nikomatsakis commented Oct 3, 2018

bors commented Oct 3, 2018

bors commented Oct 4, 2018

nikomatsakis commented Oct 4, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018

bors commented Oct 4, 2018