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

regression: non-defining opaque type use in defining scope #105826

Closed
digama0 opened this issue Dec 17, 2022 · 5 comments · Fixed by #106503
Closed

regression: non-defining opaque type use in defining scope #105826

digama0 opened this issue Dec 17, 2022 · 5 comments · Fixed by #106503
Assignees
Labels
C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Milestone

Comments

@digama0
Copy link
Contributor

digama0 commented Dec 17, 2022

Here is an example which uses a RPIT on a borrowed type in three different ways:

use std::io::Write;

struct A(Vec<u8>);

struct B<'a> {
  one: &'a mut A,
  two: &'a mut Vec<u8>,
  three: Vec<u8>,
}

impl<'a> B<'a> {
  fn one(&mut self) -> &mut impl Write { &mut self.one.0 }
  fn two(&mut self) -> &mut impl Write { &mut self.two } // fail
  fn three(&mut self) -> &mut impl Write { &mut self.three }
}

struct C<'a>(B<'a>);

impl<'a> C<'a> {
  fn one(&mut self) -> &mut impl Write { self.0.one() } // fail on beta
  fn two(&mut self) -> &mut impl Write { self.0.two() } // fail on beta
  fn three(&mut self) -> &mut impl Write { self.0.three() } // fail on beta
}

IMO this is perfectly reasonable code and there should be no errors (EDIT: see below). On stable (rustc 1.66.0 (69f9c33d7 2022-12-12)) we have the following:

error[E0700]: hidden type for `impl std::io::Write` captures lifetime that does not appear in bounds
  --> src/main.rs:13:42
   |
11 | impl<'a> B<'a> {
   |      -- hidden type `&'a mut Vec<u8>` captures the lifetime `'a` as defined here
12 |   fn one(&mut self) -> &mut impl Write { &mut self.one.0 }
13 |   fn two(&mut self) -> &mut impl Write { &mut self.two } // fail
   |                                          ^^^^^^^^^^^^^

And on beta (rustc 1.67.0-beta.2 (352eb59a4 2022-12-13)) and nightly (rustc 1.68.0-nightly (b70baa4f9 2022-12-14)) we get some additional errors:

error[E0700]: hidden type for `impl std::io::Write` captures lifetime that does not appear in bounds
  --> src/main.rs:13:42
   |
11 | impl<'a> B<'a> {
   |      -- hidden type `&'a mut Vec<u8>` captures the lifetime `'a` as defined here
12 |   fn one(&mut self) -> &mut impl Write { &mut self.one.0 }
13 |   fn two(&mut self) -> &mut impl Write { &mut self.two } // fail
   |                                          ^^^^^^^^^^^^^

error: non-defining opaque type use in defining scope
  --> src/main.rs:20:42
   |
20 |   fn one(&mut self) -> &mut impl Write { self.0.one() } // fail on beta
   |                                          ^^^^^^^^^^^^ lifetime `'a` is part of concrete type but not used in parameter list of the `impl Trait` type alias

error: non-defining opaque type use in defining scope
  --> src/main.rs:21:42
   |
21 |   fn two(&mut self) -> &mut impl Write { self.0.two() } // fail on beta
   |                                          ^^^^^^^^^^^^ lifetime `'a` is part of concrete type but not used in parameter list of the `impl Trait` type alias

error: non-defining opaque type use in defining scope
  --> src/main.rs:22:44
   |
22 |   fn three(&mut self) -> &mut impl Write { self.0.three() } // fail on beta
   |                                            ^^^^^^^^^^^^^^ lifetime `'a` is part of concrete type but not used in parameter list of the `impl Trait` type alias

EDIT: On second thought, I guess the first error (on stable) is reasonable, since we are actually returning a &mut &'a mut Vec<u8> in that case. If you either use &mut *self.two in the body or &mut (impl Write + 'a) in the return type then the error goes away. But the second error is still a regression.

Version it worked on

It most recently worked on: rustc 1.66.0 (69f9c33 2022-12-12)

Version with regression

rustc --version --verbose:

rustc 1.67.0-beta.2 (352eb59a4 2022-12-13)
binary: rustc
commit-hash: 352eb59a4c33abf739914422f2ad975925750146
commit-date: 2022-12-13
host: x86_64-unknown-linux-gnu
release: 1.67.0-beta.2
LLVM version: 15.0.6

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@digama0 digama0 added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Dec 17, 2022
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Dec 17, 2022
@aliemjay
Copy link
Member

This is #103491. Cc @cjgillot. It is related to the invariance:

fn absurd<T>() -> T { todo!() }
fn one<'a: 'a>() -> *mut impl Sized {
    absurd::<*mut ()>()
}
fn two<'a: 'a>() -> *mut impl Sized {
    one::<'a>()
    //~^ ERROR non-defining opaque type use in defining scope
}

Another case that doesn't rely on member constraints in the second function and yields a different error message:

fn absurd<T>() -> T { todo!() }
fn assert_static<T: 'static>(_: T) {}
fn one<'a: 'a>() -> *mut impl Sized {
    absurd::<*mut ()>()
}
fn two<'a>() {
    assert_static(one::<'a>());
    //~^ ERROR lifetime may not live long enough
}

I also found a distantly related regression that doesn't require invariance. It looks like a missed subtyping opportunity:

fn one<'a, 'b: 'b>() -> &'a impl Sized {
    &()
}
fn two<'a, 'b>() {
    one::<'a, 'b>();
    //~^ ERROR lifetime may not live long enough
}

@aliemjay
Copy link
Member

aliemjay commented Dec 17, 2022

I also found a distantly related regression that doesn't require invariance. It looks like a missed subtyping opportunity:

Another version using TAIT:

#![feature(type_alias_impl_trait)]
type Opaque<'a> = impl Sized;
fn define<'a>() -> Opaque<'a> {}
fn test<'a>() {
    None::<&'static Opaque<'a>>;
}

At this point there are two classes of regression: The first is related to the fact that Invariant * Bivariant = Invariant and the the second requires more subtyping relations.

@rustbot label I-types-nominated T-types T-compiler

@rustbot rustbot added the I-types-nominated Nominated for discussion during a types team meeting. label Dec 17, 2022
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Dec 22, 2022
@Mark-Simulacrum Mark-Simulacrum added this to the 1.67.0 milestone Jan 1, 2023
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Jan 5, 2023
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 10, 2023
Do not filter substs in `remap_generic_params_to_declaration_params`.

The relevant filtering should have been performed by borrowck.

Fixes rust-lang#105826

r? types
@lcnr lcnr removed the I-types-nominated Nominated for discussion during a types team meeting. label Jan 18, 2023
@lcnr
Copy link
Contributor

lcnr commented Jan 18, 2023

unnominating as it will get fixed by #106503 which is approved again. Have to try to get this into beta though, only have 1 more day for that

@bors bors closed this as completed in 6d46b1e Jan 18, 2023
bors added a commit to rust-lang/miri that referenced this issue Jan 23, 2023
Do not filter substs in `remap_generic_params_to_declaration_params`.

The relevant filtering should have been performed by borrowck.

Fixes rust-lang/rust#105826

r? types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.

7 participants