Move binder for dyn to each list item #79945

jackh726 · 2020-12-11T20:12:36Z

This essentially changes ty::Binder<&'tcx List<ExistentialTraitRef>> to &'tcx List<ty::Binder<ExistentialTraitRef>>.

This is a first step in moving the dyn Trait representation closer to Chalk, which we've talked about in @rust-lang/wg-traits.

r? @nikomatsakis

jackh726 · 2020-12-11T20:49:57Z

compiler/rustc_middle/src/ty/context.rs

@@ -87,7 +87,7 @@ pub struct CtxtInterners<'tcx> {
    substs: InternedSet<'tcx, InternalSubsts<'tcx>>,
    canonical_var_infos: InternedSet<'tcx, List<CanonicalVarInfo<'tcx>>>,
    region: InternedSet<'tcx, RegionKind>,
-    existential_predicates: InternedSet<'tcx, List<ExistentialPredicate<'tcx>>>,
+    poly_existential_predicates: InternedSet<'tcx, List<ty::Binder<ExistentialPredicate<'tcx>>>>,


This name change probably isn't necessary, but I would like to hear thoughts.

nikomatsakis · 2020-12-12T10:39:05Z

compiler/rustc_middle/src/ty/relate.rs

@@ -616,24 +616,28 @@ impl<'tcx> Relate<'tcx> for &'tcx ty::List<ty::ExistentialPredicate<'tcx>> {
        // in `a`.
        let mut a_v: Vec<_> = a.into_iter().collect();
        let mut b_v: Vec<_> = b.into_iter().collect();
-        a_v.sort_by(|a, b| a.stable_cmp(tcx, b));
+        a_v.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder()));


This seems fine, but I think it works only because we are moving towards a "normalized" form where bound things are referenced purely by index, and this binder has exactly one bound item.

Might be worth adding a comment about this.

(In particular, if we had something like for<'a>... and for<'b>..., and the late-bound regions referenced them by name, those are equivalent, but they might be sorted differently.)

nikomatsakis · 2020-12-12T10:45:20Z

r=me with comment

nikomatsakis · 2020-12-12T10:45:27Z

@bors delegate+

bors · 2020-12-12T10:45:29Z

✌️ @jackh726 can now approve this pull request

jackh726 · 2020-12-14T18:40:04Z

Alright, so the trivial explanation here is simply that stable_cmp doesn't take into account anything other than the type of predicates. In a perfect world, I would go into more depth about the scenarios in which it did, but this actually is a somewhat complicated issue (see Zulip discussion here), and it's not entirely clear that there could be multiple different bound vars here anyways.

Given that, @bors r=nikomatsakis

bors · 2020-12-14T18:40:05Z

📌 Commit 01c2520 has been approved by nikomatsakis

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `@rust-lang/wg-traits.` r? `@nikomatsakis`

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in ``@rust-lang/wg-traits.`` r? ``@nikomatsakis``

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in ```@rust-lang/wg-traits.``` r? ```@nikomatsakis```

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in ````@rust-lang/wg-traits.```` r? ````@nikomatsakis````

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `````@rust-lang/wg-traits.````` r? `````@nikomatsakis`````

…ikomatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in ``````@rust-lang/wg-traits.`````` r? ``````@nikomatsakis``````

jackh726 · 2020-12-17T00:55:39Z

@bors r-

NLL test needs to be blessed (this is causing the rollups to fail)

jackh726 · 2020-12-17T01:18:17Z

src/test/ui/issues/issue-40000.nll.stderr

+error: higher-ranked subtype error
+  --> $DIR/issue-40000.rs:6:9
+   |
+LL |     foo(bar);
+   |         ^^^
+


Not exactly sure why this particular error gets duplicated. But, this is a pretty unhelpful error anyways and other tests have similar duplication for it (sometimes multiple). So, any fix for this duplication should probably be part of a larger, unrelated fix.

jackh726 · 2020-12-17T02:08:29Z

Test has been blessed. As I said in comment above, this particular error, while duplicated, isn't helpful, is duplicated in other tests, and a fix is an orthogonal change.

@bors r=nikomatsakis

bors · 2020-12-17T02:08:31Z

📌 Commit 2edd301 has been approved by nikomatsakis

bors · 2020-12-17T18:21:25Z

⌛ Testing commit 2edd301 with merge eb4fc71...

bors · 2020-12-17T21:13:45Z

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing eb4fc71 to master...

…omatsakis Move binder for dyn to each list item This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`. This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `@rust-lang/wg-traits.` r? `@nikomatsakis`

Rustup to rust-lang/rust#79945

Move binder for dyn to each list item

ed80815

rust-highfive assigned nikomatsakis Dec 11, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2020

jackh726 commented Dec 11, 2020

View reviewed changes

nikomatsakis approved these changes Dec 12, 2020

View reviewed changes

Add explanation for skip_binder in relate

01c2520

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 Dec 14, 2020

Dylan-DPC-zz mentioned this pull request Dec 15, 2020

Rollup of 10 pull requests #80061

Closed

This was referenced Dec 15, 2020

Rollup of 10 pull requests #80063

Closed

Rollup of 11 pull requests #80064

Closed

Dylan-DPC-zz mentioned this pull request Dec 16, 2020

Rollup of 10 pull requests #80078

Closed

Dylan-DPC-zz mentioned this pull request Dec 16, 2020

Rollup of 10 pull requests #80084

Closed

Dylan-DPC-zz mentioned this pull request Dec 16, 2020

Rollup of 11 pull requests #80093

Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 17, 2020

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 17, 2020

Bless NLL test

2edd301

jackh726 commented Dec 17, 2020

View reviewed changes

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 Dec 17, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 17, 2020

bors merged commit eb4fc71 into rust-lang:master Dec 17, 2020

rustbot added this to the 1.50.0 milestone Dec 17, 2020

jackh726 deleted the existential_trait_ref branch December 17, 2020 21:14

tmiasko mentioned this pull request Mar 28, 2021

v0 mangled symbol doesn't match specification nor can it be demangled with rustfilt or c++filt #83611

Closed

JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Mar 31, 2021

Update to nightly-2020-12-19

7c43d03

Rustup to rust-lang/rust#79945

Move binder for dyn to each list item #79945

Move binder for dyn to each list item #79945

Uh oh!

Conversation

jackh726 commented Dec 11, 2020

Uh oh!

jackh726 Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

nikomatsakis Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Dec 12, 2020

Uh oh!

nikomatsakis commented Dec 12, 2020

Uh oh!

bors commented Dec 12, 2020

Uh oh!

jackh726 commented Dec 14, 2020

Uh oh!

bors commented Dec 14, 2020

Uh oh!

jackh726 commented Dec 17, 2020

Uh oh!

jackh726 Dec 17, 2020

Choose a reason for hiding this comment

Uh oh!

jackh726 commented Dec 17, 2020

Uh oh!

bors commented Dec 17, 2020

Uh oh!

bors commented Dec 17, 2020

Uh oh!

bors commented Dec 17, 2020

Uh oh!

Uh oh!