Skip to content
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

Make PolyTraitRef::self_ty return Binder<Ty> #72508

Merged
merged 2 commits into from
Jun 7, 2020

Conversation

ecstatic-morse
Copy link
Contributor

This came up during review of #71618. The current implementation is the same as a call to skip_binder but harder to audit. Make it preserve binding levels and add a call to skip_binder at all use sites so they can be audited as part of #72507.

@ecstatic-morse
Copy link
Contributor Author

r? @nikomatsakis

@ecstatic-morse ecstatic-morse changed the title Make PolyTraitPredicate::self_ty return Binder<Ty> Make PolyTraitRef::self_ty return Binder<Ty> May 23, 2020
pub fn self_ty(&self) -> Ty<'tcx> {
self.skip_binder().self_ty()
pub fn self_ty(&self) -> Binder<Ty<'tcx>> {
self.map_bound_ref(|tr| tr.self_ty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay

@@ -289,7 +289,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
(
Some(format!(
"`?` couldn't convert the error to `{}`",
trait_ref.self_ty(),
trait_ref.skip_binder().self_ty(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Less yay" -- this seems ok for now I guess but I wish we had a different solution than skipping the binder

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 26, 2020

📌 Commit 09cd547 has been approved by nikomatsakis

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 26, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 27, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
@Dylan-DPC-zz
Copy link

Dylan-DPC-zz commented May 27, 2020

failed in rollup ci check

@bors r-

@bors 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 May 27, 2020
@ecstatic-morse
Copy link
Contributor Author

@bors retry

@bors 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 Jun 2, 2020
@ecstatic-morse
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 4, 2020

📌 Commit b4e06b9 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 4, 2020

⌛ Testing commit b4e06b9 with merge 0372fd6c280a1a0b743a22403c39bb95daebf364...

@Dylan-DPC-zz
Copy link

@bors retry yield

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 5, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 5, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
@RalfJung
Copy link
Member

RalfJung commented Jun 6, 2020

Could this have caused #73043 (comment)? I can't see a direct connection, and the panic unfortunately doesn't have a useful line number, but the other changes in that rollup seem even less likely to be the culprit.

@RalfJung
Copy link
Member

RalfJung commented Jun 6, 2020

Hm, probably it's not this one, as #73053 has the same failure.

RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 6, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 6, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 6, 2020
Rollup of 3 pull requests

Successful merges:

 - rust-lang#71796 (de-promote Duration::from_secs)
 - rust-lang#72508 (Make `PolyTraitRef::self_ty` return `Binder<Ty>`)
 - rust-lang#72708 (linker: Add a linker rerun hack for gcc versions not supporting -static-pie)

Failed merges:

r? @ghost
@bors bors merged commit de4d5ce into rust-lang:master Jun 7, 2020
tesuji pushed a commit to tesuji/rustc that referenced this pull request Jun 9, 2020
…matsakis

Make `PolyTraitRef::self_ty` return `Binder<Ty>`

This came up during review of rust-lang#71618. The current implementation is the same as a call to `skip_binder` but harder to audit. Make it preserve binding levels and add a call to `skip_binder` at all use sites so they can be audited as part of rust-lang#72507.
@ecstatic-morse ecstatic-morse deleted the poly-self-ty branch October 6, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants