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

rework opaque type region inference #116891

Merged
merged 12 commits into from
Mar 28, 2024
32 changes: 20 additions & 12 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,11 @@ pub struct RegionInferenceContext<'tcx> {
/// visible from this index.
scc_universes: IndexVec<ConstraintSccIndex, ty::UniverseIndex>,

/// Contains a "representative" from each SCC. This will be the
/// minimal RegionVid belonging to that universe. It is used as a
/// kind of hacky way to manage checking outlives relationships,
/// Contains the "representative" region of each SCC.
/// It is defined as the one with the minimal RegionVid, favoring
/// free regions, then placeholders, then existential regions.
///
/// It is a hacky way to manage checking regions for equality,
/// since we can 'canonicalize' each region to the representative
/// of its SCC and be sure that -- if they have the same repr --
/// they *must* be equal (though not having the same repr does not
Expand Down Expand Up @@ -481,22 +483,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
scc_universes
}

/// For each SCC, we compute a unique `RegionVid` (in fact, the
/// minimal one that belongs to the SCC). See
/// For each SCC, we compute a unique `RegionVid`. See the
/// `scc_representatives` field of `RegionInferenceContext` for
/// more details.
fn compute_scc_representatives(
constraints_scc: &Sccs<RegionVid, ConstraintSccIndex>,
definitions: &IndexSlice<RegionVid, RegionDefinition<'tcx>>,
) -> IndexVec<ConstraintSccIndex, ty::RegionVid> {
let num_sccs = constraints_scc.num_sccs();
let next_region_vid = definitions.next_index();
let mut scc_representatives = IndexVec::from_elem_n(next_region_vid, num_sccs);

for region_vid in definitions.indices() {
let scc = constraints_scc.scc(region_vid);
let prev_min = scc_representatives[scc];
scc_representatives[scc] = region_vid.min(prev_min);
let mut scc_representatives = IndexVec::from_elem_n(RegionVid::MAX, num_sccs);

// Iterate over all RegionVids *in-order* and pick the least RegionVid as the
// representative of its SCC. This naturally prefers free regions over others.
for (vid, def) in definitions.iter_enumerated() {
let repr = &mut scc_representatives[constraints_scc.scc(vid)];
if *repr == ty::RegionVid::MAX {
*repr = vid;
} else if matches!(def.origin, NllRegionVariableOrigin::Placeholder(_))
&& matches!(definitions[*repr].origin, NllRegionVariableOrigin::Existential { .. })
{
// Pick placeholders over existentials even if they have a greater RegionVid.
*repr = vid;
}
}

scc_representatives
Expand Down
379 changes: 210 additions & 169 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ impl UniversalRegionRelations<'_> {
self.outlives.contains(fr1, fr2)
}

/// Returns `true` if fr1 is known to equal fr2.
///
/// This will only ever be true for universally quantified regions.
pub(crate) fn equal(&self, fr1: RegionVid, fr2: RegionVid) -> bool {
self.outlives.contains(fr1, fr2) && self.outlives.contains(fr2, fr1)
}

/// Returns a vector of free regions `x` such that `fr1: x` is
/// known to hold.
pub(crate) fn regions_outlived_by(&self, fr1: RegionVid) -> Vec<RegionVid> {
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,22 @@ pub(crate) fn type_check<'mir, 'tcx>(
);
}

// Convert all regions to nll vars.
let (opaque_type_key, hidden_type) =
infcx.tcx.fold_regions((opaque_type_key, hidden_type), |region, _| {
match region.kind() {
ty::ReVar(_) => region,
ty::RePlaceholder(placeholder) => checker
.borrowck_context
.constraints
.placeholder_region(infcx, placeholder),
_ => ty::Region::new_var(
infcx.tcx,
checker.borrowck_context.universal_regions.to_region_vid(region),
),
}
});

(opaque_type_key, hidden_type)
})
.collect();
Expand Down
32 changes: 32 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,38 @@ pub struct OpaqueTypeKey<'tcx> {
pub args: GenericArgsRef<'tcx>,
}

impl<'tcx> OpaqueTypeKey<'tcx> {
pub fn iter_captured_args(
self,
tcx: TyCtxt<'tcx>,
) -> impl Iterator<Item = (usize, GenericArg<'tcx>)> {
std::iter::zip(self.args, tcx.variances_of(self.def_id)).enumerate().filter_map(
|(i, (arg, v))| match (arg.unpack(), v) {
(_, ty::Invariant) => Some((i, arg)),
(ty::GenericArgKind::Lifetime(_), ty::Bivariant) => None,
_ => bug!("unexpected opaque type arg variance"),
},
)
}

pub fn fold_captured_lifetime_args(
self,
tcx: TyCtxt<'tcx>,
mut f: impl FnMut(Region<'tcx>) -> Region<'tcx>,
) -> Self {
let Self { def_id, args } = self;
let args = std::iter::zip(args, tcx.variances_of(def_id)).map(|(arg, v)| {
match (arg.unpack(), v) {
(ty::GenericArgKind::Lifetime(_), ty::Bivariant) => arg,
(ty::GenericArgKind::Lifetime(lt), _) => f(lt).into(),
_ => arg,
}
});
let args = tcx.mk_args_from_iter(args);
Self { def_id, args }
}
}

#[derive(Copy, Clone, Debug, TypeFoldable, TypeVisitable, HashStable, TyEncodable, TyDecodable)]
pub struct OpaqueHiddenType<'tcx> {
/// The span of this particular definition of the opaque type. So
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0792]: expected generic lifetime parameter, found `'_`
--> $DIR/defining-use-captured-non-universal-region.rs:13:18
|
LL | fn foo<'a>() -> impl Sized + 'a {
| -- this generic parameter must be used with a generic lifetime parameter
...
LL | let i: i32 = foo::<'_>();
| ^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0792`.
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/defining-use-captured-non-universal-region.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// This was an ICE. See #110726.

//@ revisions: statik infer fixed
//@ [fixed] check-pass
#![allow(unconditional_recursion)]

fn foo<'a>() -> impl Sized + 'a {
#[cfg(statik)]
let i: i32 = foo::<'static>();
//[statik]~^ ERROR expected generic lifetime parameter, found `'static`

#[cfg(infer)]
let i: i32 = foo::<'_>();
//[infer]~^ ERROR expected generic lifetime parameter, found `'_`

#[cfg(fixed)]
let i: i32 = foo::<'a>();

i
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0792]: expected generic lifetime parameter, found `'static`
--> $DIR/defining-use-captured-non-universal-region.rs:9:18
|
LL | fn foo<'a>() -> impl Sized + 'a {
| -- cannot use static lifetime; use a bound lifetime instead or remove the lifetime parameter from the opaque type
LL | #[cfg(statik)]
LL | let i: i32 = foo::<'static>();
| ^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0792`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// issue: #110623
//@ check-pass

use std::{collections::BTreeMap, num::ParseIntError, str::FromStr};

enum FileSystem {
File(usize),
Directory(BTreeMap<String, FileSystem>),
}

impl FromStr for FileSystem {
type Err = ParseIntError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.starts_with("dir") {
Ok(Self::new_dir())
} else {
Ok(Self::File(s.split_whitespace().next().unwrap().parse()?))
}
}
}

impl FileSystem {
fn new_dir() -> FileSystem {
FileSystem::Directory(BTreeMap::new())
}

fn insert(&mut self, name: String, other: FileSystem) -> Option<FileSystem> {
match self {
FileSystem::File(_) => panic!("can only insert into directory!"),
FileSystem::Directory(tree) => tree.insert(name, other),
}
}

// Recursively build a tree from commands. This uses (abuses?)
// the fact that `cd /` only appears at the start and that
// subsequent `cd`s can only move ONE level to use the recursion
// stack as the filesystem stack
fn build<'a>(
&mut self,
mut commands: impl Iterator<Item = &'a str> + 'a,
) -> Option<impl Iterator<Item = &'a str> + 'a> {
let cmd = commands.next()?;
let mut elements = cmd.lines();
match elements.next().map(str::trim) {
Some("cd /") | None => self.build(commands),
Some("cd ..") => {
// return to higher scope
Some(commands)
}
Some("ls") => {
for item in elements {
let name = item.split_whitespace().last().unwrap();
let prior = self.insert(name.to_string(), item.parse().unwrap());
debug_assert!(prior.is_none());
}
// continue on
self.build(commands)
}
Some(other_cd) => {
let name = other_cd
.trim()
.strip_prefix("cd ")
.expect("expected a cd command");
let mut directory = FileSystem::new_dir();
let further_commands = directory.build(commands);
self.insert(name.to_string(), directory);
self.build(further_commands?) // THIS LINE FAILS TO COMPILE
}
}
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ check-pass

#![feature(adt_const_params)]
#![allow(incomplete_features)]

trait Bar<const FOO: &'static str> {}
impl Bar<"asdf"> for () {}

fn foo<const FOO: &'static str>() -> impl Bar<"asdf"> {
()
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// issue: #111906
//@ check-pass

#![allow(unconditional_recursion)]

fn foo<'a: 'a>() -> impl Sized {
let _: () = foo::<'a>();
loop {}
}

fn bar<'a: 'a>() -> impl Sized + 'a {
let _: *mut &'a () = bar::<'a>();
loop {}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: {foo<DefId(..)_'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
error: {foo<'{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
--> $DIR/erased-regions-in-hidden-ty.rs:12:36
|
LL | fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: {foo<DefId(..)_'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
error: {foo<'{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
--> $DIR/erased-regions-in-hidden-ty.rs:12:36
|
LL | fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/impl-trait/erased-regions-in-hidden-ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// Make sure that the compiler can handle `ReErased` in the hidden type of an opaque.

fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {
//~^ ERROR 'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
//~^ ERROR '{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
// Can't write whole type because of lack of path sanitization
|| ()
}
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/impl-trait/impl-fn-predefined-lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
use std::fmt::Debug;

fn a<'a>() -> impl Fn(&'a u8) -> (impl Debug + '_) {
//~^ ERROR cannot resolve opaque type

|x| x
//~^ ERROR concrete type differs from previous defining opaque type use
//~^ ERROR expected generic lifetime parameter, found `'_`
}

fn _b<'a>() -> impl Fn(&'a u8) -> (impl Debug + 'a) {
Expand Down
24 changes: 7 additions & 17 deletions tests/ui/impl-trait/impl-fn-predefined-lifetimes.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
error: concrete type differs from previous defining opaque type use
--> $DIR/impl-fn-predefined-lifetimes.rs:7:9
|
LL | |x| x
| ^ expected `impl Debug + '_`, got `&u8`
|
note: previous use here
--> $DIR/impl-fn-predefined-lifetimes.rs:7:5
|
LL | |x| x
| ^^^^^

error[E0720]: cannot resolve opaque type
--> $DIR/impl-fn-predefined-lifetimes.rs:4:35
error[E0792]: expected generic lifetime parameter, found `'_`
--> $DIR/impl-fn-predefined-lifetimes.rs:5:9
|
LL | fn a<'a>() -> impl Fn(&'a u8) -> (impl Debug + '_) {
| ^^^^^^^^^^^^^^^ cannot resolve opaque type
| -- this generic parameter must be used with a generic lifetime parameter
LL | |x| x
| ^

error: aborting due to 2 previous errors
error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0720`.
For more information about this error, try `rustc --explain E0792`.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ trait Foo {
fn bar<'other: 'a>() -> impl Sized + 'a {}
//~^ ERROR use of undeclared lifetime name `'a`
//~| ERROR use of undeclared lifetime name `'a`
//~| ERROR expected generic lifetime parameter, found `'static`
}

fn main() {}
13 changes: 11 additions & 2 deletions tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ help: consider introducing lifetime `'a` here
LL | trait Foo<'a> {
| ++++

error: aborting due to 2 previous errors
error[E0792]: expected generic lifetime parameter, found `'static`
--> $DIR/bad-item-bound-within-rpitit-2.rs:5:45
|
LL | fn bar<'other: 'a>() -> impl Sized + 'a {}
| ------ ^^
| |
| cannot use static lifetime; use a bound lifetime instead or remove the lifetime parameter from the opaque type

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0261`.
Some errors have detailed explanations: E0261, E0792.
For more information about an error, try `rustc --explain E0261`.
1 change: 1 addition & 0 deletions tests/ui/impl-trait/rpit/early_bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ fn test<'a: 'a>(n: bool) -> impl Sized + 'a {
let true = n else { loop {} };
let _ = || {
let _ = identity::<&'a ()>(test(false));
//~^ ERROR expected generic lifetime parameter, found `'_`
};
loop {}
}
Expand Down
Loading
Loading