From 3ef7f6878e267b0be5736373cf04f9d54497f1b2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 19 Oct 2023 08:26:32 +0000 Subject: [PATCH] Prevent opaque types being instantiated twice with different regions within the same function --- compiler/rustc_borrowck/messages.ftl | 6 ++ .../src/region_infer/opaque_types.rs | 96 +++++++++++++++---- .../rustc_borrowck/src/session_diagnostics.rs | 13 +++ src/tools/tidy/src/issues.txt | 1 - tests/ui/impl-trait/issue-86465.rs | 10 -- tests/ui/impl-trait/issue-86465.stderr | 11 --- .../lifetime_mismatch.rs | 22 +++++ .../lifetime_mismatch.stderr | 47 +++++++++ .../multiple-def-uses-in-one-fn-lifetimes.rs | 4 +- ...ltiple-def-uses-in-one-fn-lifetimes.stderr | 29 +++++- .../multiple-def-uses-in-one-fn-pass.rs | 10 +- .../type-alias-impl-trait/param_mismatch.rs | 16 ++++ .../param_mismatch.stderr | 12 +++ .../type-alias-impl-trait/param_mismatch2.rs | 16 ++++ .../param_mismatch2.stderr | 12 +++ .../type-alias-impl-trait/param_mismatch3.rs | 26 +++++ .../param_mismatch3.stderr | 21 ++++ 17 files changed, 298 insertions(+), 54 deletions(-) delete mode 100644 tests/ui/impl-trait/issue-86465.rs delete mode 100644 tests/ui/impl-trait/issue-86465.stderr create mode 100644 tests/ui/type-alias-impl-trait/lifetime_mismatch.rs create mode 100644 tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch.rs create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch.stderr create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch2.rs create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch2.stderr create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch3.rs create mode 100644 tests/ui/type-alias-impl-trait/param_mismatch3.stderr diff --git a/compiler/rustc_borrowck/messages.ftl b/compiler/rustc_borrowck/messages.ftl index bf14d5eb9a0a8..1c3192d19cd8c 100644 --- a/compiler/rustc_borrowck/messages.ftl +++ b/compiler/rustc_borrowck/messages.ftl @@ -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") -> diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 4b096a592344a..d478828661c4a 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -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> { + 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 { + 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, 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. /// @@ -65,6 +125,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { infcx: &InferCtxt<'tcx>, opaque_ty_decls: FxIndexMap, OpaqueHiddenType<'tcx>>, ) -> FxIndexMap> { + self.check_unique(infcx, &opaque_ty_decls); + let mut result: FxIndexMap> = FxIndexMap::default(); let member_constraints: FxIndexMap<_, _> = self @@ -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", + ) } }; diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index a055ce95e8ef3..5957ef0efde7a 100644 --- a/compiler/rustc_borrowck/src/session_diagnostics.rs +++ b/compiler/rustc_borrowck/src/session_diagnostics.rs @@ -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)] diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt index 51af8898470b3..9addc90434126 100644 --- a/src/tools/tidy/src/issues.txt +++ b/src/tools/tidy/src/issues.txt @@ -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", diff --git a/tests/ui/impl-trait/issue-86465.rs b/tests/ui/impl-trait/issue-86465.rs deleted file mode 100644 index a79bb6474d8ba..0000000000000 --- a/tests/ui/impl-trait/issue-86465.rs +++ /dev/null @@ -1,10 +0,0 @@ -#![feature(type_alias_impl_trait)] - -type X<'a, 'b> = impl std::fmt::Debug; - -fn f<'t, 'u>(a: &'t u32, b: &'u u32) -> (X<'t, 'u>, X<'u, 't>) { - (a, a) - //~^ ERROR concrete type differs from previous defining opaque type use -} - -fn main() {} diff --git a/tests/ui/impl-trait/issue-86465.stderr b/tests/ui/impl-trait/issue-86465.stderr deleted file mode 100644 index e330d178d4eac..0000000000000 --- a/tests/ui/impl-trait/issue-86465.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: concrete type differs from previous defining opaque type use - --> $DIR/issue-86465.rs:6:5 - | -LL | (a, a) - | ^^^^^^ - | | - | expected `&'a u32`, got `&'b u32` - | this expression supplies two conflicting concrete types for the same opaque type - -error: aborting due to 1 previous error - diff --git a/tests/ui/type-alias-impl-trait/lifetime_mismatch.rs b/tests/ui/type-alias-impl-trait/lifetime_mismatch.rs new file mode 100644 index 0000000000000..de62710f96552 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/lifetime_mismatch.rs @@ -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)); +} diff --git a/tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr b/tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr new file mode 100644 index 0000000000000..d25b20447a442 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr @@ -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 + diff --git a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs index 3f122f1060956..5fc8e563cefad 100644 --- a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs +++ b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs @@ -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() {} diff --git a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr index 552cf3fda3000..38b920e316c5b 100644 --- a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr +++ b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr @@ -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 diff --git a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs index 40c00e553a6df..aba41a9d85235 100644 --- a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs +++ b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs @@ -7,15 +7,11 @@ fn f(a: A, b: B) -> (X, X) (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!("{}", as ToString>::to_string(&f(42_i32, String::new()).1)); - let meh = 42; - let muh = 69; - println!("{:?}", foo(&meh, &muh)); } diff --git a/tests/ui/type-alias-impl-trait/param_mismatch.rs b/tests/ui/type-alias-impl-trait/param_mismatch.rs new file mode 100644 index 0000000000000..c746503071376 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch.rs @@ -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() {} diff --git a/tests/ui/type-alias-impl-trait/param_mismatch.stderr b/tests/ui/type-alias-impl-trait/param_mismatch.stderr new file mode 100644 index 0000000000000..09ec550d71834 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch.stderr @@ -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`. diff --git a/tests/ui/type-alias-impl-trait/param_mismatch2.rs b/tests/ui/type-alias-impl-trait/param_mismatch2.rs new file mode 100644 index 0000000000000..c7d5eaa16aaec --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch2.rs @@ -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() {} diff --git a/tests/ui/type-alias-impl-trait/param_mismatch2.stderr b/tests/ui/type-alias-impl-trait/param_mismatch2.stderr new file mode 100644 index 0000000000000..1ecdd7c2b54c6 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch2.stderr @@ -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`. diff --git a/tests/ui/type-alias-impl-trait/param_mismatch3.rs b/tests/ui/type-alias-impl-trait/param_mismatch3.rs new file mode 100644 index 0000000000000..03c133d5d3c2c --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch3.rs @@ -0,0 +1,26 @@ +//! 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 id2<'a, 'b>(s: (&'a str, &'b str)) -> (&'a str, &'b str) { + s +} + +type Opaque<'a> = impl Sized + 'a; + +fn test() -> impl for<'a, 'b> Fn((&'a str, &'b str)) -> (Opaque<'a>, Opaque<'b>) { + id2 //~ ERROR expected generic lifetime parameter, found `'a` +} + +fn id(s: &str) -> &str { + s +} + +type Opaque2<'a> = impl Sized + 'a; + +fn test2(s: &str) -> (impl Fn(&str) -> Opaque2<'_>, Opaque2<'_>) { + (id, s) //~ ERROR: expected generic lifetime parameter, found `'_` +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/param_mismatch3.stderr b/tests/ui/type-alias-impl-trait/param_mismatch3.stderr new file mode 100644 index 0000000000000..b8805f9b7f650 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/param_mismatch3.stderr @@ -0,0 +1,21 @@ +error[E0792]: expected generic lifetime parameter, found `'a` + --> $DIR/param_mismatch3.rs:13:5 + | +LL | type Opaque<'a> = impl Sized + 'a; + | -- this generic parameter must be used with a generic lifetime parameter +... +LL | id2 + | ^^^ + +error[E0792]: expected generic lifetime parameter, found `'_` + --> $DIR/param_mismatch3.rs:23:5 + | +LL | type Opaque2<'a> = impl Sized + 'a; + | -- this generic parameter must be used with a generic lifetime parameter +... +LL | (id, s) + | ^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0792`.