Skip to content

Commit

Permalink
Prevent opaque types being instantiated twice with different regions …
Browse files Browse the repository at this point in the history
…within the same function
  • Loading branch information
oli-obk committed Feb 28, 2024
1 parent ef32456 commit 3ef7f68
Show file tree
Hide file tree
Showing 17 changed files with 298 additions and 54 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_borrowck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ borrowck_moved_due_to_usage_in_operator =
*[false] operator
}
borrowck_opaque_type_lifetime_mismatch =
opaque type used twice with different lifetimes
.label = lifetime `{$arg}` used here
.prev_lifetime_label = lifetime `{$prev}` previously used here
.note = if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
borrowck_opaque_type_non_generic_param =
expected generic {$kind} parameter, found `{$ty}`
.label = {STREQ($ty, "'static") ->
Expand Down
96 changes: 76 additions & 20 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,77 @@ use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_macros::extension;
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::RegionVid;
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{GenericArgKind, GenericArgs};
use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::ObligationCtxt;

use crate::session_diagnostics::LifetimeMismatchOpaqueParam;
use crate::session_diagnostics::NonGenericOpaqueTypeParam;

use super::RegionInferenceContext;

impl<'tcx> RegionInferenceContext<'tcx> {
fn universal_name(&self, vid: ty::RegionVid) -> Option<ty::Region<'tcx>> {
let scc = self.constraint_sccs.scc(vid);
self.scc_values
.universal_regions_outlived_by(scc)
.find_map(|lb| self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?))
}

fn generic_arg_to_region(&self, arg: ty::GenericArg<'tcx>) -> Option<RegionVid> {
let region = arg.as_region()?;

if let ty::RePlaceholder(..) = region.kind() {
None
} else {
Some(self.to_region_vid(region))
}
}

/// Check that all opaque types have the same region parameters if they have the same
/// non-region parameters. This is necessary because within the new solver we perform various query operations
/// modulo regions, and thus could unsoundly select some impls that don't hold.
fn check_unique(
&self,
infcx: &InferCtxt<'tcx>,
opaque_ty_decls: &FxIndexMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
) {
for (i, (a, a_ty)) in opaque_ty_decls.iter().enumerate() {
for (b, b_ty) in opaque_ty_decls.iter().skip(i + 1) {
if a.def_id != b.def_id {
continue;
}
// Non-lifetime params differ -> ok
if infcx.tcx.erase_regions(a.args) != infcx.tcx.erase_regions(b.args) {
continue;
}
trace!(?a, ?b);
for (a, b) in a.args.iter().zip(b.args) {
trace!(?a, ?b);
let Some(r1) = self.generic_arg_to_region(a) else {
continue;
};
let Some(r2) = self.generic_arg_to_region(b) else {
continue;
};
if self.eval_equal(r1, r2) {
continue;
}

infcx.dcx().emit_err(LifetimeMismatchOpaqueParam {
arg: self.universal_name(r1).unwrap().into(),
prev: self.universal_name(r2).unwrap().into(),
span: a_ty.span,
prev_span: b_ty.span,
});
}
}
}
}

/// Resolve any opaque types that were encountered while borrow checking
/// this item. This is then used to get the type in the `type_of` query.
///
Expand Down Expand Up @@ -65,6 +125,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
infcx: &InferCtxt<'tcx>,
opaque_ty_decls: FxIndexMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
) -> FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> {
self.check_unique(infcx, &opaque_ty_decls);

let mut result: FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> = FxIndexMap::default();

let member_constraints: FxIndexMap<_, _> = self
Expand All @@ -80,26 +142,20 @@ impl<'tcx> RegionInferenceContext<'tcx> {

let mut arg_regions = vec![self.universal_regions.fr_static];

let to_universal_region = |vid, arg_regions: &mut Vec<_>| {
trace!(?vid);
let scc = self.constraint_sccs.scc(vid);
trace!(?scc);
match self.scc_values.universal_regions_outlived_by(scc).find_map(|lb| {
self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?)
}) {
Some(region) => {
let vid = self.universal_regions.to_region_vid(region);
arg_regions.push(vid);
region
}
None => {
arg_regions.push(vid);
ty::Region::new_error_with_message(
infcx.tcx,
concrete_type.span,
"opaque type with non-universal region args",
)
}
let to_universal_region = |vid, arg_regions: &mut Vec<_>| match self.universal_name(vid)
{
Some(region) => {
let vid = self.universal_regions.to_region_vid(region);
arg_regions.push(vid);
region
}
None => {
arg_regions.push(vid);
ty::Region::new_error_with_message(
infcx.tcx,
concrete_type.span,
"opaque type with non-universal region args",
)
}
};

Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,19 @@ pub(crate) struct NonGenericOpaqueTypeParam<'a, 'tcx> {
pub param_span: Span,
}

#[derive(Diagnostic)]
#[diag(borrowck_opaque_type_lifetime_mismatch)]
pub(crate) struct LifetimeMismatchOpaqueParam<'tcx> {
pub arg: GenericArg<'tcx>,
pub prev: GenericArg<'tcx>,
#[primary_span]
#[label]
#[note]
pub span: Span,
#[label(borrowck_prev_lifetime_label)]
pub prev_span: Span,
}

#[derive(Subdiagnostic)]
pub(crate) enum CaptureReasonLabel<'a> {
#[label(borrowck_moved_due_to_call)]
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,6 @@
"ui/impl-trait/issue-56445.rs",
"ui/impl-trait/issue-68532.rs",
"ui/impl-trait/issue-72911.rs",
"ui/impl-trait/issue-86465.rs",
"ui/impl-trait/issue-87450.rs",
"ui/impl-trait/issue-99073-2.rs",
"ui/impl-trait/issue-99073.rs",
Expand Down
10 changes: 0 additions & 10 deletions tests/ui/impl-trait/issue-86465.rs

This file was deleted.

11 changes: 0 additions & 11 deletions tests/ui/impl-trait/issue-86465.stderr

This file was deleted.

22 changes: 22 additions & 0 deletions tests/ui/type-alias-impl-trait/lifetime_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![feature(type_alias_impl_trait)]

type Foo<'a> = impl Sized;

fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> (Foo<'a>, Foo<'b>) {
(x, y)
//~^ ERROR opaque type used twice with different lifetimes
}

type Bar<'a, 'b> = impl std::fmt::Debug;

fn bar<'x, 'y>(i: &'x i32, j: &'y i32) -> (Bar<'x, 'y>, Bar<'y, 'x>) {
(i, j)
//~^ ERROR opaque type used twice with different lifetimes
//~| ERROR opaque type used twice with different lifetimes
}

fn main() {
let meh = 42;
let muh = 69;
println!("{:?}", bar(&meh, &muh));
}
47 changes: 47 additions & 0 deletions tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: opaque type used twice with different lifetimes
--> $DIR/lifetime_mismatch.rs:6:5
|
LL | (x, y)
| ^^^^^^
| |
| lifetime `'a` used here
| lifetime `'b` previously used here
|
note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
--> $DIR/lifetime_mismatch.rs:6:5
|
LL | (x, y)
| ^^^^^^

error: opaque type used twice with different lifetimes
--> $DIR/lifetime_mismatch.rs:13:5
|
LL | (i, j)
| ^^^^^^
| |
| lifetime `'x` used here
| lifetime `'y` previously used here
|
note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
--> $DIR/lifetime_mismatch.rs:13:5
|
LL | (i, j)
| ^^^^^^

error: opaque type used twice with different lifetimes
--> $DIR/lifetime_mismatch.rs:13:5
|
LL | (i, j)
| ^^^^^^
| |
| lifetime `'y` used here
| lifetime `'x` previously used here
|
note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
--> $DIR/lifetime_mismatch.rs:13:5
|
LL | (i, j)
| ^^^^^^

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
type Foo<'a, 'b> = impl std::fmt::Debug;

fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) {
(i, i) //~ ERROR concrete type differs from previous
(i, i)
//~^ ERROR opaque type used twice with different lifetimes
//~| ERROR opaque type used twice with different lifetimes
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,11 +1,32 @@
error: concrete type differs from previous defining opaque type use
error: opaque type used twice with different lifetimes
--> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
|
LL | (i, i)
| ^^^^^^
| |
| expected `&'a i32`, got `&'b i32`
| this expression supplies two conflicting concrete types for the same opaque type
| lifetime `'x` used here
| lifetime `'y` previously used here
|
note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
--> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
|
LL | (i, i)
| ^^^^^^

error: opaque type used twice with different lifetimes
--> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
|
LL | (i, i)
| ^^^^^^
| |
| lifetime `'y` used here
| lifetime `'x` previously used here
|
note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types
--> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
|
LL | (i, i)
| ^^^^^^

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

Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@ fn f<A: ToString + Clone, B: ToString + Clone>(a: A, b: B) -> (X<A, B>, X<A, B>)
(a.clone(), a)
}

type Foo<'a, 'b> = impl std::fmt::Debug;

fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) {
(i, j)
type Tait<'x> = impl Sized;
fn define<'a: 'b, 'b: 'a>(x: &'a u8, y: &'b u8) -> (Tait<'a>, Tait<'b>) {
((), ())
}

fn main() {
println!("{}", <X<_, _> as ToString>::to_string(&f(42_i32, String::new()).1));
let meh = 42;
let muh = 69;
println!("{:?}", foo(&meh, &muh));
}
16 changes: 16 additions & 0 deletions tests/ui/type-alias-impl-trait/param_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//! This test checks that when checking for opaque types that
//! only differ in lifetimes, we handle the case of non-generic
//! regions correctly.
#![feature(type_alias_impl_trait)]

fn id(s: &str) -> &str {
s
}
type Opaque<'a> = impl Sized + 'a;
// The second `Opaque<'_>` has a higher kinded lifetime, not a generic parameter
fn test(s: &str) -> (Opaque<'_>, impl Fn(&str) -> Opaque<'_>) {
(s, id)
//~^ ERROR: expected generic lifetime parameter, found `'_`
}

fn main() {}
12 changes: 12 additions & 0 deletions tests/ui/type-alias-impl-trait/param_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0792]: expected generic lifetime parameter, found `'_`
--> $DIR/param_mismatch.rs:12:5
|
LL | type Opaque<'a> = impl Sized + 'a;
| -- this generic parameter must be used with a generic lifetime parameter
...
LL | (s, id)
| ^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0792`.
16 changes: 16 additions & 0 deletions tests/ui/type-alias-impl-trait/param_mismatch2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//! This test checks that when checking for opaque types that
//! only differ in lifetimes, we handle the case of non-generic
//! regions correctly.
#![feature(type_alias_impl_trait)]

fn id(s: &str) -> &str {
s
}

type Opaque<'a> = impl Sized + 'a;

fn test(s: &str) -> (impl Fn(&str) -> Opaque<'_>, impl Fn(&str) -> Opaque<'_>) {
(id, id) //~ ERROR: expected generic lifetime parameter, found `'_`
}

fn main() {}
12 changes: 12 additions & 0 deletions tests/ui/type-alias-impl-trait/param_mismatch2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0792]: expected generic lifetime parameter, found `'_`
--> $DIR/param_mismatch2.rs:13:5
|
LL | type Opaque<'a> = impl Sized + 'a;
| -- this generic parameter must be used with a generic lifetime parameter
...
LL | (id, id)
| ^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0792`.
Loading

0 comments on commit 3ef7f68

Please sign in to comment.