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

opaque type definition: strict lifetime equality vs equal-by-inference #113971

Closed
aliemjay opened this issue Jul 23, 2023 · 0 comments · Fixed by #116891
Closed

opaque type definition: strict lifetime equality vs equal-by-inference #113971

aliemjay opened this issue Jul 23, 2023 · 0 comments · Fixed by #116891
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@aliemjay
Copy link
Member

aliemjay commented Jul 23, 2023

The following code compiles but I don't think it should:

#![feature(type_alias_impl_trait)]
trait Captures<'a> {}
impl<T> Captures<'_> for T {}

fn ensure_outlives<'a, X: 'a>(_: X) {}
fn relate<X>(_: X, _: X) {}

type Opaque<'a> = impl Copy + Captures<'a>;
fn test<'x>(_: Opaque<'x>) {
    let opaque = None::<Opaque<'_>>; // let's call this lifetime '?1
    let hidden = None::<u8>;
    ensure_outlives::<'x>(opaque); // outlives constraint: '?1: 'x
    relate(opaque, hidden); // defining use: Opaque<'?1> := u8
}

Here we have a defining use of an opaque type Opaque<'_> == u8 with a questionable legality. Let's call the inferred lifetime '?1.

At each defining use of an opaque type, we require its lifetime arguments (['?1]) to be equal to one of the lifetime parameters in scope (in this case there is only 'x). This defining use passes this check because ensure_outlives registers a region constraint '?1: 'x, and, because we infer the least possible value for lifetime variables, we end up inferring '?1 == 'x.

The current situation is fine in regard to borrowck soundness (which is the reason we have such check in the first place) but it requires a subtle reasoning of lifetimes and makes the code more fragile (more on that later). For these reason, I believe we should enforce a stricter form of equality that requires both constraints ['x: '?1, '?1: 'x] in order to consider the lifetimes equal. It would be backward-compatible to change that later.

To demonstrate how fragile the current behavior can be, here is a couple of trivial changes that break the original example.

  • '?1: 'x constraint is no longer generated at ensure_equal:
-type Opaque<'a> = impl Copy + Captures<'a>;
+type Opaque<'a> = impl Copy + Captures<'a> + 'static;
  • a fundamental limitation of member constraints is exposed. I believe it is hard to fix.
     let opaque = None::<Opaque<'_>>;
-    let hidden = None::<u8>;
+    let hidden = None::<&'x u8>;

cc @rust-lang/nitiative-impl-trait

@aliemjay aliemjay added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue. labels Jul 23, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 23, 2023
@aliemjay aliemjay removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 23, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 19, 2023
…2, r=<try>

rework opaque type region inference

fixes rust-lang#113971 Pass -> Error

fixes rust-lang#111906 ICE -> Pass
fixes rust-lang#110623 ==
fixes rust-lang#109059 ==

fixes rust-lang#112841 Pass -> Error

fixes rust-lang#110726 ICE->Error

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 21, 2023
…2, r=<try>

rework opaque type region inference

fixes rust-lang#113971 Pass -> Error

fixes rust-lang#111906 ICE -> Pass
fixes rust-lang#110623 ==
fixes rust-lang#109059 ==

fixes rust-lang#112841 Pass -> Error

fixes rust-lang#110726 ICE->Error

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 23, 2023
…2, r=<try>

rework opaque type region inference

fixes rust-lang#113971 Pass -> Error

fixes rust-lang#111906 ICE -> Pass
fixes rust-lang#110623 ==
fixes rust-lang#109059 ==

fixes rust-lang#112841 Pass -> Error

fixes rust-lang#110726 ICE->Error

fixes rust-lang#111935 Pass -> Error
fixes rust-lang#113916 ==

r? `@ghost`
@bors bors closed this as completed in 551abd6 Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants