Skip to content

Commit

Permalink
Auto merge of #116891 - aliemjay:opaque-region-infer-rework-2, r=comp…
Browse files Browse the repository at this point in the history
…iler-errors,oli-obk

rework opaque type region inference

User-facing changes are documented in [this comment](#116891 (comment)).

The design document is in [this comment](#116891 (comment)).

---

\- Fix Ice in check_unique; ICE -> Error; fixes #122782.
\- Ignore uncaptured lifetime args; ICE -> Pass; fixes #111906, fixes #110623, fixes #109059, fixes #122307
\- Except equal parameters from the uniqueness check; Pass -> Error; fixes #113916.
\- Check RPITs for invalid args; Pass -> Error; fixes #111935; ICE -> Error; fixes #110726.
\- Rework opaque types region inference; Pass -> Error; fixes #113971, fixes #112841.
\- Reject external lifetimes as invalid args; Pass -> Error; fixes #105498.

r? `@ghost`
  • Loading branch information
bors committed Mar 28, 2024
2 parents d0e8cbb + 59c217f commit 551abd6
Show file tree
Hide file tree
Showing 40 changed files with 879 additions and 345 deletions.
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 @@ -833,6 +833,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

0 comments on commit 551abd6

Please sign in to comment.