From 9b1fceec36b1cd2acc64907ce963afdc5882f7cf Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 19 Oct 2023 08:26:32 +0000 Subject: [PATCH 1/2] 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 | 71 ++++++++++++++++--- .../rustc_borrowck/src/session_diagnostics.rs | 13 ++++ 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 | 9 --- 10 files changed, 178 insertions(+), 44 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 diff --git a/compiler/rustc_borrowck/messages.ftl b/compiler/rustc_borrowck/messages.ftl index 2c7b97afa8011..6c3142ff7df96 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 ff04b0237c244..7545ff894d5af 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -13,11 +13,68 @@ 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 gen_arg_to_name(&self, arg: ty::GenericArg<'tcx>) -> Option> { + let region = arg.as_region()?; + + if let ty::RePlaceholder(..) = region.kind() { + return None; + } + self.universal_name(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) { + if a.as_region().is_none() { + break; + } + let a = self.gen_arg_to_name(a).unwrap(); + let b = self.gen_arg_to_name(b).unwrap(); + trace!(?a, ?b); + if a == b { + continue; + } + + infcx.tcx.sess.emit_err(LifetimeMismatchOpaqueParam { + arg: a.into(), + prev: b.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. /// @@ -63,6 +120,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 @@ -78,13 +137,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mut subst_regions = vec![self.universal_regions.fr_static]; - let to_universal_region = |vid, subst_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?) - }) { + let to_universal_region = + |vid, subst_regions: &mut Vec<_>| match self.universal_name(vid) { Some(region) => { let vid = self.universal_regions.to_region_vid(region); subst_regions.push(vid); @@ -98,8 +152,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { "opaque type with non-universal region args", ) } - } - }; + }; // Start by inserting universal regions from the member_constraint choice regions. // This will ensure they get precedence when folding the regions in the concrete type. diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index ca3ccf439f280..53de351fae631 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/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 90d6904ed6164..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 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..276bcfd2e2a24 --- /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 81e603e2355df..a1d8b6e88cc1d 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 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 83fd9a1da450b..f412b2d0e7db7 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,6 @@ 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) -} - fn main() { println!("{}", as ToString>::to_string(&f(42_i32, String::new()).1)); - let meh = 42; - let muh = 69; - println!("{:?}", foo(&meh, &muh)); } From 29b10dca0d5f50cc1c6c021ad0b21b250cc692e2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 20 Oct 2023 15:28:43 +0000 Subject: [PATCH 2/2] Properly check for lifetime equality instead of relying on identity --- .../src/region_infer/opaque_types.rs | 23 ++++++++++--------- .../multiple-def-uses-in-one-fn-pass.rs | 5 ++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 7545ff894d5af..1015f3bc51c9c 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -7,6 +7,7 @@ use rustc_infer::infer::TyCtxtInferExt as _; use rustc_infer::traits::{Obligation, ObligationCause}; 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; @@ -26,13 +27,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { .find_map(|lb| self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?)) } - fn gen_arg_to_name(&self, arg: ty::GenericArg<'tcx>) -> Option> { + fn generic_arg_to_region(&self, arg: ty::GenericArg<'tcx>) -> Option { let region = arg.as_region()?; if let ty::RePlaceholder(..) = region.kind() { - return None; + None + } else { + Some(self.to_region_vid(region)) } - self.universal_name(self.to_region_vid(region)) } /// Check that all opaque types have the same region parameters if they have the same @@ -54,19 +56,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { } trace!(?a, ?b); for (a, b) in a.args.iter().zip(b.args) { - if a.as_region().is_none() { - break; - } - let a = self.gen_arg_to_name(a).unwrap(); - let b = self.gen_arg_to_name(b).unwrap(); trace!(?a, ?b); - if a == b { + let Some(r1) = self.generic_arg_to_region(a) else { + continue; + }; + let r2 = self.generic_arg_to_region(b).unwrap(); + if self.eval_equal(r1, r2) { continue; } infcx.tcx.sess.emit_err(LifetimeMismatchOpaqueParam { - arg: a.into(), - prev: b.into(), + arg: self.universal_name(r1).unwrap().into(), + prev: self.universal_name(r2).unwrap().into(), span: a_ty.span, prev_span: b_ty.span, }); 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 f412b2d0e7db7..c9c585ac7bc5a 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,6 +7,11 @@ fn f(a: A, b: B) -> (X, X) (a.clone(), a) } +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)); }