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

Audit uses of skip_binder in diagnostics code #72507

Open
ecstatic-morse opened this issue May 23, 2020 · 0 comments
Open

Audit uses of skip_binder in diagnostics code #72507

ecstatic-morse opened this issue May 23, 2020 · 0 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-traits Area: Trait system C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented May 23, 2020

As discussed in #71618, skip_binder is called very often in diagnostics code. In general, this is incorrect for types with late-bound regions, e.g. function pointers. We should audit uses of skip_binder and replace them with no_bound_vars + early return or unwrap. If there are uses of skip_binder that are benign, for example debug printing or comparing with a known type, we should abstract this behind a function to minimize the number of calls to skip_binder that need to be checked.

@ecstatic-morse ecstatic-morse added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-traits Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 23, 2020
@jonas-schievink jonas-schievink added the A-diagnostics Area: Messages for errors, warnings, and lints label May 23, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue 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 pushed a commit to Dylan-DPC-zz/rust that referenced this issue 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 issue 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 added a commit to RalfJung/rust that referenced this issue 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 issue 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 issue 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.
tesuji pushed a commit to tesuji/rustc that referenced this issue 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-traits Area: Trait system C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants