From c8979e587b1851560f030065a21f28456899825a Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 07:40:17 +0000 Subject: [PATCH 01/13] Move `opaque_type_origin_unchecked` onto `TyCtxt` and re-use it where it was open coded --- compiler/rustc_hir_analysis/src/check/check.rs | 4 ++-- compiler/rustc_hir_analysis/src/check/wfcheck.rs | 4 ++-- compiler/rustc_infer/src/infer/opaque_types.rs | 9 +-------- compiler/rustc_metadata/src/rmeta/encoder.rs | 4 ++-- compiler/rustc_middle/src/ty/context.rs | 6 ++++++ compiler/rustc_ty_utils/src/assoc.rs | 2 +- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 22aa3c7ff08b4..a7e81f41c6996 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -563,8 +563,8 @@ fn check_item_type(tcx: TyCtxt<'_>, id: hir::ItemId) { check_union(tcx, id.owner_id.def_id); } DefKind::OpaqueTy => { - let opaque = tcx.hir().expect_item(id.owner_id.def_id).expect_opaque_ty(); - if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = opaque.origin + let origin = tcx.opaque_type_origin(id.owner_id.def_id); + if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = origin && let hir::Node::TraitItem(trait_item) = tcx.hir().get_by_def_id(fn_def_id) && let (_, hir::TraitFn::Required(..)) = trait_item.expect_fn() { diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 51e9b4c4501d8..f654a67831334 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -1543,8 +1543,8 @@ impl<'tcx> TypeVisitor> for ImplTraitInTraitFinder<'_, 'tcx> { if let ty::Alias(ty::Opaque, unshifted_opaque_ty) = *ty.kind() && self.seen.insert(unshifted_opaque_ty.def_id) && let Some(opaque_def_id) = unshifted_opaque_ty.def_id.as_local() - && let opaque = tcx.hir().expect_item(opaque_def_id).expect_opaque_ty() - && let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = opaque.origin + && let origin = tcx.opaque_type_origin(opaque_def_id) + && let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = origin && source == self.fn_def_id { let opaque_ty = tcx.fold_regions(unshifted_opaque_ty, |re, _depth| { diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index 3cb1788dc194a..c5659ffd5c763 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -378,7 +378,7 @@ impl<'tcx> InferCtxt<'tcx> { DefiningAnchor::Bind(bind) => bind, }; - let origin = self.opaque_type_origin_unchecked(def_id); + let origin = self.tcx.opaque_type_origin(def_id); let in_definition_scope = match origin { // Async `impl Trait` hir::OpaqueTyOrigin::AsyncFn(parent) => parent == parent_def_id, @@ -395,13 +395,6 @@ impl<'tcx> InferCtxt<'tcx> { }; in_definition_scope.then_some(origin) } - - /// Returns the origin of the opaque type `def_id` even if we are not in its - /// defining scope. - #[instrument(skip(self), level = "trace", ret)] - fn opaque_type_origin_unchecked(&self, def_id: LocalDefId) -> OpaqueTyOrigin { - self.tcx.hir().expect_item(def_id).expect_opaque_ty().origin - } } /// Visitor that requires that (almost) all regions in the type visited outlive diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 12f3a0fc2c215..eee29d2090a82 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1138,8 +1138,8 @@ fn should_encode_type(tcx: TyCtxt<'_>, def_id: LocalDefId, def_kind: DefKind) -> | DefKind::InlineConst => true, DefKind::OpaqueTy => { - let opaque = tcx.hir().expect_item(def_id).expect_opaque_ty(); - if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = opaque.origin + let origin = tcx.opaque_type_origin(def_id); + if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = origin && let hir::Node::TraitItem(trait_item) = tcx.hir().get_by_def_id(fn_def_id) && let (_, hir::TraitFn::Required(..)) = trait_item.expect_fn() { diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 2b4f69167bfb0..a123869be64a8 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1189,6 +1189,12 @@ impl<'tcx> TyCtxt<'tcx> { pub fn local_visibility(self, def_id: LocalDefId) -> Visibility { self.visibility(def_id).expect_local() } + + /// Returns the origin of the opaque type `def_id`. + #[instrument(skip(self), level = "trace", ret)] + pub fn opaque_type_origin(self, def_id: LocalDefId) -> hir::OpaqueTyOrigin { + self.hir().expect_item(def_id).expect_opaque_ty().origin + } } /// A trait implemented for all `X<'a>` types that can be safely and diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs index 658ab03c0f48c..5b731641e9d53 100644 --- a/compiler/rustc_ty_utils/src/assoc.rs +++ b/compiler/rustc_ty_utils/src/assoc.rs @@ -259,7 +259,7 @@ fn associated_type_for_impl_trait_in_trait( opaque_ty_def_id: LocalDefId, ) -> LocalDefId { let (hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id)) = - tcx.hir().expect_item(opaque_ty_def_id).expect_opaque_ty().origin + tcx.opaque_type_origin(opaque_ty_def_id) else { bug!("expected opaque for {opaque_ty_def_id:?}"); }; From bae645451e5bc3e5cec77a92b4888915356c28ba Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 14:10:42 +0000 Subject: [PATCH 02/13] Only create the opaque collector once and visit it many times --- compiler/rustc_ty_utils/src/opaque_types.rs | 30 +++++++-------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 34c7b9f445182..7af5f847e8b6b 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -22,14 +22,8 @@ struct OpaqueTypeCollector<'tcx> { } impl<'tcx> OpaqueTypeCollector<'tcx> { - fn collect( - tcx: TyCtxt<'tcx>, - item: LocalDefId, - val: ty::Binder<'tcx, impl TypeVisitable>>, - ) -> Vec { - let mut collector = Self { tcx, opaques: Vec::new(), item, seen: Default::default() }; - val.skip_binder().visit_with(&mut collector); - collector.opaques + fn new(tcx: TyCtxt<'tcx>, item: LocalDefId) -> Self { + Self { tcx, opaques: Vec::new(), item, seen: Default::default() } } fn span(&self) -> Span { @@ -166,21 +160,17 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [ match kind { // We're also doing this for `AssocTy` for the wf checks in `check_opaque_meets_bounds` DefKind::Fn | DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => { - let defined_opaques = match kind { - DefKind::Fn => { - OpaqueTypeCollector::collect(tcx, item, tcx.fn_sig(item).subst_identity()) + let mut collector = OpaqueTypeCollector::new(tcx, item); + match kind { + DefKind::AssocFn | DefKind::Fn => { + tcx.fn_sig(item).subst_identity().visit_with(&mut collector); } - DefKind::AssocFn => { - OpaqueTypeCollector::collect(tcx, item, tcx.fn_sig(item).subst_identity()) + DefKind::AssocTy | DefKind::AssocConst => { + tcx.type_of(item).subst_identity().visit_with(&mut collector); } - DefKind::AssocTy | DefKind::AssocConst => OpaqueTypeCollector::collect( - tcx, - item, - ty::Binder::dummy(tcx.type_of(item).subst_identity()), - ), _ => unreachable!(), - }; - tcx.arena.alloc_from_iter(defined_opaques) + } + tcx.arena.alloc_from_iter(collector.opaques) } DefKind::Mod | DefKind::Struct From 12243ec41586bd5b68c032ef456a369fb0350469 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 14:34:32 +0000 Subject: [PATCH 03/13] Point to argument/return type instead of the whole function header --- .../infer/error_reporting/note_and_explain.rs | 3 +- compiler/rustc_ty_utils/src/opaque_types.rs | 36 +++++++++++++++---- .../issue-88595.stderr | 8 ++--- .../in-assoc-type-unconstrained.stderr | 4 +-- tests/ui/impl-trait/in-assoc-type.stderr | 4 +-- ...s-impl-trait-declaration-too-subtle.stderr | 8 ++--- .../higher_kinded_params3.rs | 35 ++++++++++++++++++ .../higher_kinded_params3.stderr | 15 ++++++++ .../invalid_impl_trait_in_assoc_ty.stderr | 4 +-- ...t-matching-trait-refs-isnt-defining.stderr | 4 +-- .../unnameable_type.stderr | 4 +-- 11 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 tests/ui/type-alias-impl-trait/higher_kinded_params3.rs create mode 100644 tests/ui/type-alias-impl-trait/higher_kinded_params3.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index a723dc3f079d7..2a32f0b504737 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -260,7 +260,8 @@ impl Trait for X { (ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) if alias.def_id.is_local() && matches!(tcx.def_kind(body_owner_def_id), DefKind::AssocFn | DefKind::AssocConst) => { if tcx.is_type_alias_impl_trait(alias.def_id) { if !tcx.opaque_types_defined_by(body_owner_def_id.expect_local()).contains(&alias.def_id.expect_local()) { - diag.span_note(tcx.def_span(body_owner_def_id), "\ + let sp = tcx.def_ident_span(body_owner_def_id).unwrap_or_else(|| tcx.def_span(body_owner_def_id)); + diag.span_note(sp, "\ this item must have the opaque type in its signature \ in order to be able to register hidden types"); } diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 7af5f847e8b6b..d97b74017ed62 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -19,15 +19,26 @@ struct OpaqueTypeCollector<'tcx> { /// Avoid infinite recursion due to recursive declarations. seen: FxHashSet, + + span: Option, } impl<'tcx> OpaqueTypeCollector<'tcx> { fn new(tcx: TyCtxt<'tcx>, item: LocalDefId) -> Self { - Self { tcx, opaques: Vec::new(), item, seen: Default::default() } + Self { tcx, opaques: Vec::new(), item, seen: Default::default(), span: None } } fn span(&self) -> Span { - self.tcx.def_span(self.item) + self.span.unwrap_or_else(|| { + self.tcx.def_ident_span(self.item).unwrap_or_else(|| self.tcx.def_span(self.item)) + }) + } + + fn visit_spanned(&mut self, span: Span, value: impl TypeVisitable>) { + let old = self.span; + self.span = Some(span); + value.visit_with(self); + self.span = old; } fn parent_trait_ref(&self) -> Option> { @@ -72,13 +83,13 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { self.opaques.push(alias_ty.def_id.expect_local()); // Collect opaque types nested within the associated type bounds of this opaque type. - for (pred, _span) in self + for (pred, span) in self .tcx .explicit_item_bounds(alias_ty.def_id) .subst_iter_copied(self.tcx, alias_ty.substs) { trace!(?pred); - pred.visit_with(self)?; + self.visit_spanned(span, pred); } ControlFlow::Continue(()) @@ -163,10 +174,23 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [ let mut collector = OpaqueTypeCollector::new(tcx, item); match kind { DefKind::AssocFn | DefKind::Fn => { - tcx.fn_sig(item).subst_identity().visit_with(&mut collector); + let ty_sig = tcx.fn_sig(item).subst_identity(); + let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap(); + collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output()); + for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) { + collector.visit_spanned(hir.span, ty.map_bound(|x| *x)); + } } DefKind::AssocTy | DefKind::AssocConst => { - tcx.type_of(item).subst_identity().visit_with(&mut collector); + let span = match tcx.hir().get_by_def_id(item) { + rustc_hir::Node::ImplItem(it) => match it.kind { + rustc_hir::ImplItemKind::Const(ty, _) => ty.span, + rustc_hir::ImplItemKind::Type(ty) => ty.span, + other => span_bug!(tcx.def_span(item), "{other:#?}"), + }, + other => span_bug!(tcx.def_span(item), "{other:#?}"), + }; + collector.visit_spanned(span, tcx.type_of(item).subst_identity()); } _ => unreachable!(), } diff --git a/tests/ui/generic-associated-types/issue-88595.stderr b/tests/ui/generic-associated-types/issue-88595.stderr index d6caed8545993..b0f6864f6ec1a 100644 --- a/tests/ui/generic-associated-types/issue-88595.stderr +++ b/tests/ui/generic-associated-types/issue-88595.stderr @@ -1,8 +1,8 @@ error: non-defining opaque type use in defining scope - --> $DIR/issue-88595.rs:21:5 + --> $DIR/issue-88595.rs:21:23 | LL | fn a(&'a self) -> Self::B<'a> {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ generic argument `'a` used twice + | ^^^^^^^^^^^ generic argument `'a` used twice | note: for this opaque type --> $DIR/issue-88595.rs:19:18 @@ -24,10 +24,10 @@ LL | fn a(&'a self) -> Self::B<'a> {} = note: expected opaque type `>::B<'a>` found unit type `()` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/issue-88595.rs:21:5 + --> $DIR/issue-88595.rs:21:8 | LL | fn a(&'a self) -> Self::B<'a> {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ error: aborting due to 2 previous errors diff --git a/tests/ui/impl-trait/in-assoc-type-unconstrained.stderr b/tests/ui/impl-trait/in-assoc-type-unconstrained.stderr index 1097cd0f452a8..8e61a65abe4be 100644 --- a/tests/ui/impl-trait/in-assoc-type-unconstrained.stderr +++ b/tests/ui/impl-trait/in-assoc-type-unconstrained.stderr @@ -40,10 +40,10 @@ LL | fn method() -> Self::Ty; = note: expected signature `fn() -> <() as compare_method::Trait>::Ty` found signature `fn()` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/in-assoc-type-unconstrained.rs:22:9 + --> $DIR/in-assoc-type-unconstrained.rs:22:12 | LL | fn method() -> () {} - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^ error: unconstrained opaque type --> $DIR/in-assoc-type-unconstrained.rs:20:19 diff --git a/tests/ui/impl-trait/in-assoc-type.stderr b/tests/ui/impl-trait/in-assoc-type.stderr index f0a272dc2d5d1..ab3f3a14410d0 100644 --- a/tests/ui/impl-trait/in-assoc-type.stderr +++ b/tests/ui/impl-trait/in-assoc-type.stderr @@ -12,10 +12,10 @@ LL | fn foo(&self) -> >::Bar {} = note: expected opaque type `<() as Foo<()>>::Bar` found unit type `()` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/in-assoc-type.rs:17:5 + --> $DIR/in-assoc-type.rs:17:8 | LL | fn foo(&self) -> >::Bar {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^ error: aborting due to previous error diff --git a/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr b/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr index bbd60d4398b74..3e1981f096c77 100644 --- a/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr +++ b/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr @@ -21,10 +21,10 @@ LL | fn eq(&self, _other: &(Foo, i32)) -> bool { = note: expected signature `fn(&a::Bar, &(a::Bar, i32)) -> _` found signature `fn(&a::Bar, &(a::Foo, i32)) -> _` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:10:9 + --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:10:12 | LL | fn eq(&self, _other: &(Foo, i32)) -> bool { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^ error: unconstrained opaque type --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:18:16 @@ -49,10 +49,10 @@ LL | fn eq(&self, _other: &(Bar, i32)) -> bool { = note: expected signature `fn(&b::Bar, &(b::Foo, i32)) -> _` found signature `fn(&b::Bar, &(b::Bar, i32)) -> _` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:24:9 + --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:24:12 | LL | fn eq(&self, _other: &(Bar, i32)) -> bool { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^ error: aborting due to 4 previous errors diff --git a/tests/ui/type-alias-impl-trait/higher_kinded_params3.rs b/tests/ui/type-alias-impl-trait/higher_kinded_params3.rs new file mode 100644 index 0000000000000..839a611cb71a9 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/higher_kinded_params3.rs @@ -0,0 +1,35 @@ +//! This test checks that we can't actually have an opaque type behind +//! a binder that references variables from that binder. + +// edition: 2021 + +#![feature(type_alias_impl_trait)] + +trait B { + type C; +} + +struct A; + +impl<'a> B for &'a A { + type C = Tait<'a>; +} + +type Tait<'a> = impl std::fmt::Debug + 'a; + +struct Terminator; + +type Successors<'a> = impl std::fmt::Debug + 'a; + +impl Terminator { + fn successors(&self, mut f: for<'x> fn(&'x ()) -> <&'x A as B>::C) -> Successors<'_> { + f = g; + //~^ ERROR: mismatched types + } +} + +fn g(x: &()) -> &() { + x +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/higher_kinded_params3.stderr b/tests/ui/type-alias-impl-trait/higher_kinded_params3.stderr new file mode 100644 index 0000000000000..aaba9ad5ca704 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/higher_kinded_params3.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> $DIR/higher_kinded_params3.rs:26:9 + | +LL | type Tait<'a> = impl std::fmt::Debug + 'a; + | ------------------------- the expected opaque type +... +LL | f = g; + | ^^^^^ one type is more general than the other + | + = note: expected fn pointer `for<'x> fn(&'x ()) -> Tait<'x>` + found fn pointer `for<'a> fn(&'a ()) -> &'a ()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/invalid_impl_trait_in_assoc_ty.stderr b/tests/ui/type-alias-impl-trait/invalid_impl_trait_in_assoc_ty.stderr index 2beed73cb85c3..6ec5d13f812a7 100644 --- a/tests/ui/type-alias-impl-trait/invalid_impl_trait_in_assoc_ty.stderr +++ b/tests/ui/type-alias-impl-trait/invalid_impl_trait_in_assoc_ty.stderr @@ -12,10 +12,10 @@ LL | let x: Self::Foo = (); = note: expected opaque type `<() as Foo>::Foo` found unit type `()` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/invalid_impl_trait_in_assoc_ty.rs:10:5 + --> $DIR/invalid_impl_trait_in_assoc_ty.rs:10:8 | LL | fn bar() { - | ^^^^^^^^ + | ^^^ error: aborting due to previous error diff --git a/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr b/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr index d2d007490915b..a621bb519cdf3 100644 --- a/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr +++ b/tests/ui/type-alias-impl-trait/not-matching-trait-refs-isnt-defining.stderr @@ -12,10 +12,10 @@ LL | let _: >::Assoc = ""; = note: expected opaque type `<() as Foo>::Assoc` found reference `&'static str` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/not-matching-trait-refs-isnt-defining.rs:16:5 + --> $DIR/not-matching-trait-refs-isnt-defining.rs:16:8 | LL | fn test() -> <() as Foo>::Assoc { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ error: aborting due to previous error diff --git a/tests/ui/type-alias-impl-trait/unnameable_type.stderr b/tests/ui/type-alias-impl-trait/unnameable_type.stderr index 24f5cc8c7339b..c609ace919e5b 100644 --- a/tests/ui/type-alias-impl-trait/unnameable_type.stderr +++ b/tests/ui/type-alias-impl-trait/unnameable_type.stderr @@ -26,10 +26,10 @@ LL | fn dont_define_this(_private: Private) {} = note: expected signature `fn(Private)` found signature `fn(MyPrivate)` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/unnameable_type.rs:20:5 + --> $DIR/unnameable_type.rs:20:8 | LL | fn dont_define_this(_private: MyPrivate) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors From 326a9fa8e890902abf1f4e8aaa5be9010b366f4c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 22 Jun 2023 15:02:44 +0000 Subject: [PATCH 04/13] Add tests showcasing our short circuiting behaviour in the signature checks for defining scopes --- tests/ui/type-alias-impl-trait/multi-error.rs | 25 ++++++++++ .../type-alias-impl-trait/multi-error.stderr | 49 +++++++++++++++++++ .../non-defining-method.rs | 22 +++++++++ .../non-defining-method.stderr | 33 +++++++++++++ 4 files changed, 129 insertions(+) create mode 100644 tests/ui/type-alias-impl-trait/multi-error.rs create mode 100644 tests/ui/type-alias-impl-trait/multi-error.stderr create mode 100644 tests/ui/type-alias-impl-trait/non-defining-method.rs create mode 100644 tests/ui/type-alias-impl-trait/non-defining-method.stderr diff --git a/tests/ui/type-alias-impl-trait/multi-error.rs b/tests/ui/type-alias-impl-trait/multi-error.rs new file mode 100644 index 0000000000000..d10967abf9abb --- /dev/null +++ b/tests/ui/type-alias-impl-trait/multi-error.rs @@ -0,0 +1,25 @@ +//! This test checks that we don't follow up +//! with type mismatch errors of opaque types +//! with their hidden types if we failed the +//! defining scope check at the signature level. + +#![feature(impl_trait_in_assoc_type)] + +trait Foo { + type Bar; + type Baz; + fn foo() -> (Self::Bar, Self::Baz); +} + +impl Foo for () { + type Bar = impl Sized; + type Baz = impl Sized; + fn foo() -> (Self::Bar, Self::Baz) { + //~^ ERROR non-defining opaque type use + ((), ()) + //~^ ERROR mismatched types + //~| ERROR mismatched types + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/multi-error.stderr b/tests/ui/type-alias-impl-trait/multi-error.stderr new file mode 100644 index 0000000000000..1725937374972 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/multi-error.stderr @@ -0,0 +1,49 @@ +error: non-defining opaque type use in defining scope + --> $DIR/multi-error.rs:17:17 + | +LL | fn foo() -> (Self::Bar, Self::Baz) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument `u32` is not a generic parameter + | +note: for this opaque type + --> $DIR/multi-error.rs:15:19 + | +LL | type Bar = impl Sized; + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/multi-error.rs:19:10 + | +LL | type Bar = impl Sized; + | ---------- the expected opaque type +... +LL | ((), ()) + | ^^ expected opaque type, found `()` + | + = note: expected opaque type `<() as Foo>::Bar` + found unit type `()` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/multi-error.rs:17:8 + | +LL | fn foo() -> (Self::Bar, Self::Baz) { + | ^^^ + +error[E0308]: mismatched types + --> $DIR/multi-error.rs:19:14 + | +LL | type Baz = impl Sized; + | ---------- the expected opaque type +... +LL | ((), ()) + | ^^ expected opaque type, found `()` + | + = note: expected opaque type `<() as Foo>::Baz` + found unit type `()` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/multi-error.rs:17:8 + | +LL | fn foo() -> (Self::Bar, Self::Baz) { + | ^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/non-defining-method.rs b/tests/ui/type-alias-impl-trait/non-defining-method.rs new file mode 100644 index 0000000000000..66ceedee8f7e9 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/non-defining-method.rs @@ -0,0 +1,22 @@ +//! This test checks that we don't follow up +//! with type mismatch errors of opaque types +//! with their hidden types if we failed the +//! defining scope check at the signature level. + +#![feature(impl_trait_in_assoc_type)] + +trait Foo { + type Bar; + fn foo() -> Self::Bar; + fn bar() -> Self::Bar; +} + +impl Foo for () { + type Bar = impl Sized; + fn foo() -> Self::Bar {} + //~^ ERROR non-defining opaque type use + //~| ERROR mismatched types + fn bar() -> Self::Bar {} +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/non-defining-method.stderr b/tests/ui/type-alias-impl-trait/non-defining-method.stderr new file mode 100644 index 0000000000000..f203cff903600 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/non-defining-method.stderr @@ -0,0 +1,33 @@ +error: non-defining opaque type use in defining scope + --> $DIR/non-defining-method.rs:16:17 + | +LL | fn foo() -> Self::Bar {} + | ^^^^^^^^^^^^^^ argument `u32` is not a generic parameter + | +note: for this opaque type + --> $DIR/non-defining-method.rs:15:19 + | +LL | type Bar = impl Sized; + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/non-defining-method.rs:16:17 + | +LL | type Bar = impl Sized; + | ---------- the expected opaque type +LL | fn foo() -> Self::Bar {} + | --- ^^^^^^^^^^^^^^ expected opaque type, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | + = note: expected opaque type `<() as Foo>::Bar` + found unit type `()` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/non-defining-method.rs:16:8 + | +LL | fn foo() -> Self::Bar {} + | ^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 41881aece2aac46b21c509dadb0b101da1f7d6bb Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 15:40:31 +0000 Subject: [PATCH 05/13] Stop failing eagerly, and collect all opaque types even if some are erroneous. --- compiler/rustc_ty_utils/src/opaque_types.rs | 18 ++++++------------ tests/ui/type-alias-impl-trait/multi-error.rs | 1 - .../type-alias-impl-trait/multi-error.stderr | 19 +------------------ 3 files changed, 7 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index d97b74017ed62..d64569db56656 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -1,5 +1,4 @@ use rustc_data_structures::fx::FxHashSet; -use rustc_errors::ErrorGuaranteed; use rustc_hir::{def::DefKind, def_id::LocalDefId}; use rustc_middle::query::Providers; use rustc_middle::ty::util::{CheckRegions, NotUniqueParam}; @@ -65,10 +64,9 @@ impl<'tcx> OpaqueTypeCollector<'tcx> { } impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { - type BreakTy = ErrorGuaranteed; - #[instrument(skip(self), ret, level = "trace")] - fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + t.super_visit_with(self)?; match t.kind() { ty::Alias(ty::Opaque, alias_ty) if alias_ty.def_id.is_local() => { if !self.seen.insert(alias_ty.def_id.expect_local()) { @@ -91,24 +89,20 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { trace!(?pred); self.visit_spanned(span, pred); } - - ControlFlow::Continue(()) } Err(NotUniqueParam::NotParam(arg)) => { - let err = self.tcx.sess.emit_err(NotParam { + self.tcx.sess.emit_err(NotParam { arg, span: self.span(), opaque_span: self.tcx.def_span(alias_ty.def_id), }); - ControlFlow::Break(err) } Err(NotUniqueParam::DuplicateParam(arg)) => { - let err = self.tcx.sess.emit_err(DuplicateArg { + self.tcx.sess.emit_err(DuplicateArg { arg, span: self.span(), opaque_span: self.tcx.def_span(alias_ty.def_id), }); - ControlFlow::Break(err) } } } @@ -157,10 +151,10 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { } } } - t.super_visit_with(self) } - _ => t.super_visit_with(self), + _ => {} } + ControlFlow::Continue(()) } } diff --git a/tests/ui/type-alias-impl-trait/multi-error.rs b/tests/ui/type-alias-impl-trait/multi-error.rs index d10967abf9abb..7cae9a12cb243 100644 --- a/tests/ui/type-alias-impl-trait/multi-error.rs +++ b/tests/ui/type-alias-impl-trait/multi-error.rs @@ -18,7 +18,6 @@ impl Foo for () { //~^ ERROR non-defining opaque type use ((), ()) //~^ ERROR mismatched types - //~| ERROR mismatched types } } diff --git a/tests/ui/type-alias-impl-trait/multi-error.stderr b/tests/ui/type-alias-impl-trait/multi-error.stderr index 1725937374972..1b7b96075be08 100644 --- a/tests/ui/type-alias-impl-trait/multi-error.stderr +++ b/tests/ui/type-alias-impl-trait/multi-error.stderr @@ -27,23 +27,6 @@ note: this item must have the opaque type in its signature in order to be able t LL | fn foo() -> (Self::Bar, Self::Baz) { | ^^^ -error[E0308]: mismatched types - --> $DIR/multi-error.rs:19:14 - | -LL | type Baz = impl Sized; - | ---------- the expected opaque type -... -LL | ((), ()) - | ^^ expected opaque type, found `()` - | - = note: expected opaque type `<() as Foo>::Baz` - found unit type `()` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/multi-error.rs:17:8 - | -LL | fn foo() -> (Self::Bar, Self::Baz) { - | ^^^ - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0308`. From a71628c1147ce8b739799e33da352b3fa11559bb Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 22 Jun 2023 15:11:07 +0000 Subject: [PATCH 06/13] Treat opaque types failing the signature defining scope check as defining, as we already errored and can hide subsequent errors this way. --- compiler/rustc_ty_utils/src/opaque_types.rs | 5 +++-- .../generic-associated-types/issue-88595.rs | 1 - .../issue-88595.stderr | 22 +------------------ tests/ui/type-alias-impl-trait/multi-error.rs | 1 - .../type-alias-impl-trait/multi-error.stderr | 20 +---------------- .../non-defining-method.rs | 1 - .../non-defining-method.stderr | 21 +----------------- 7 files changed, 6 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index d64569db56656..ab97406452e58 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -72,14 +72,15 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { if !self.seen.insert(alias_ty.def_id.expect_local()) { return ControlFlow::Continue(()); } + + self.opaques.push(alias_ty.def_id.expect_local()); + match self.tcx.uses_unique_generic_params(alias_ty.substs, CheckRegions::Bound) { Ok(()) => { // FIXME: implement higher kinded lifetime bounds on nested opaque types. They are not // supported at all, so this is sound to do, but once we want to support them, you'll // start seeing the error below. - self.opaques.push(alias_ty.def_id.expect_local()); - // Collect opaque types nested within the associated type bounds of this opaque type. for (pred, span) in self .tcx diff --git a/tests/ui/generic-associated-types/issue-88595.rs b/tests/ui/generic-associated-types/issue-88595.rs index 7de906e7ef3f3..5a40a61297233 100644 --- a/tests/ui/generic-associated-types/issue-88595.rs +++ b/tests/ui/generic-associated-types/issue-88595.rs @@ -19,5 +19,4 @@ impl<'a> A<'a> for C { type B<'b> = impl Clone; fn a(&'a self) -> Self::B<'a> {} //~ ERROR: non-defining opaque type use in defining scope - //~^ ERROR: mismatched types } diff --git a/tests/ui/generic-associated-types/issue-88595.stderr b/tests/ui/generic-associated-types/issue-88595.stderr index b0f6864f6ec1a..2b1a25acfa430 100644 --- a/tests/ui/generic-associated-types/issue-88595.stderr +++ b/tests/ui/generic-associated-types/issue-88595.stderr @@ -10,25 +10,5 @@ note: for this opaque type LL | type B<'b> = impl Clone; | ^^^^^^^^^^ -error[E0308]: mismatched types - --> $DIR/issue-88595.rs:21:23 - | -LL | type B<'b> = impl Clone; - | ---------- the expected opaque type -LL | -LL | fn a(&'a self) -> Self::B<'a> {} - | - ^^^^^^^^^^^ expected opaque type, found `()` - | | - | implicitly returns `()` as its body has no tail or `return` expression - | - = note: expected opaque type `>::B<'a>` - found unit type `()` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/issue-88595.rs:21:8 - | -LL | fn a(&'a self) -> Self::B<'a> {} - | ^ - -error: aborting due to 2 previous errors +error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/multi-error.rs b/tests/ui/type-alias-impl-trait/multi-error.rs index 7cae9a12cb243..b5ff06572d06e 100644 --- a/tests/ui/type-alias-impl-trait/multi-error.rs +++ b/tests/ui/type-alias-impl-trait/multi-error.rs @@ -17,7 +17,6 @@ impl Foo for () { fn foo() -> (Self::Bar, Self::Baz) { //~^ ERROR non-defining opaque type use ((), ()) - //~^ ERROR mismatched types } } diff --git a/tests/ui/type-alias-impl-trait/multi-error.stderr b/tests/ui/type-alias-impl-trait/multi-error.stderr index 1b7b96075be08..6d8fa0fb021a8 100644 --- a/tests/ui/type-alias-impl-trait/multi-error.stderr +++ b/tests/ui/type-alias-impl-trait/multi-error.stderr @@ -10,23 +10,5 @@ note: for this opaque type LL | type Bar = impl Sized; | ^^^^^^^^^^ -error[E0308]: mismatched types - --> $DIR/multi-error.rs:19:10 - | -LL | type Bar = impl Sized; - | ---------- the expected opaque type -... -LL | ((), ()) - | ^^ expected opaque type, found `()` - | - = note: expected opaque type `<() as Foo>::Bar` - found unit type `()` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/multi-error.rs:17:8 - | -LL | fn foo() -> (Self::Bar, Self::Baz) { - | ^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/non-defining-method.rs b/tests/ui/type-alias-impl-trait/non-defining-method.rs index 66ceedee8f7e9..2f4a7052f7221 100644 --- a/tests/ui/type-alias-impl-trait/non-defining-method.rs +++ b/tests/ui/type-alias-impl-trait/non-defining-method.rs @@ -15,7 +15,6 @@ impl Foo for () { type Bar = impl Sized; fn foo() -> Self::Bar {} //~^ ERROR non-defining opaque type use - //~| ERROR mismatched types fn bar() -> Self::Bar {} } diff --git a/tests/ui/type-alias-impl-trait/non-defining-method.stderr b/tests/ui/type-alias-impl-trait/non-defining-method.stderr index f203cff903600..61bf2da20dbdb 100644 --- a/tests/ui/type-alias-impl-trait/non-defining-method.stderr +++ b/tests/ui/type-alias-impl-trait/non-defining-method.stderr @@ -10,24 +10,5 @@ note: for this opaque type LL | type Bar = impl Sized; | ^^^^^^^^^^ -error[E0308]: mismatched types - --> $DIR/non-defining-method.rs:16:17 - | -LL | type Bar = impl Sized; - | ---------- the expected opaque type -LL | fn foo() -> Self::Bar {} - | --- ^^^^^^^^^^^^^^ expected opaque type, found `()` - | | - | implicitly returns `()` as its body has no tail or `return` expression - | - = note: expected opaque type `<() as Foo>::Bar` - found unit type `()` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/non-defining-method.rs:16:8 - | -LL | fn foo() -> Self::Bar {} - | ^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. From d6e1b206238d032a0d13fe66e316664a7c03ded3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 15:43:24 +0000 Subject: [PATCH 07/13] Fix a codegen test --- ...er-cfi-emit-type-metadata-id-itanium-cxx-abi.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs b/tests/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs index 472d921ace046..63e63c5d4aa92 100644 --- a/tests/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs +++ b/tests/codegen/sanitizer-cfi-emit-type-metadata-id-itanium-cxx-abi.rs @@ -61,7 +61,19 @@ pub type Type9 = impl Send; pub type Type10 = impl Send; pub type Type11 = impl Send; -pub fn fn1<'a>() { +pub fn fn1<'a>() where + Type1: 'static, + Type2: 'static, + Type3: 'static, + Type4: 'static, + Type5: 'static, + Type6: 'static, + Type7: 'static, + Type8: 'static, + Type9: 'static, + Type10: 'static, + Type11: 'static, +{ // Closure let closure1 = || { }; let _: Type1 = closure1; From 30ff127036b685aca52d121807d13e8e1d2db684 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 20:20:39 +0000 Subject: [PATCH 08/13] Re-use error code for duplicate error --- compiler/rustc_ty_utils/src/errors.rs | 2 +- tests/ui/type-alias-impl-trait/multi-error.stderr | 3 ++- tests/ui/type-alias-impl-trait/non-defining-method.stderr | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_ty_utils/src/errors.rs b/compiler/rustc_ty_utils/src/errors.rs index 553bf40ef3a48..947d4bbe86e1c 100644 --- a/compiler/rustc_ty_utils/src/errors.rs +++ b/compiler/rustc_ty_utils/src/errors.rs @@ -113,7 +113,7 @@ pub struct DuplicateArg<'tcx> { } #[derive(Diagnostic)] -#[diag(ty_utils_impl_trait_not_param)] +#[diag(ty_utils_impl_trait_not_param, code = "E0792")] pub struct NotParam<'tcx> { pub arg: GenericArg<'tcx>, #[primary_span] diff --git a/tests/ui/type-alias-impl-trait/multi-error.stderr b/tests/ui/type-alias-impl-trait/multi-error.stderr index 6d8fa0fb021a8..b2de2effea662 100644 --- a/tests/ui/type-alias-impl-trait/multi-error.stderr +++ b/tests/ui/type-alias-impl-trait/multi-error.stderr @@ -1,4 +1,4 @@ -error: non-defining opaque type use in defining scope +error[E0792]: non-defining opaque type use in defining scope --> $DIR/multi-error.rs:17:17 | LL | fn foo() -> (Self::Bar, Self::Baz) { @@ -12,3 +12,4 @@ LL | type Bar = impl Sized; error: aborting due to previous error +For more information about this error, try `rustc --explain E0792`. diff --git a/tests/ui/type-alias-impl-trait/non-defining-method.stderr b/tests/ui/type-alias-impl-trait/non-defining-method.stderr index 61bf2da20dbdb..ed5590f9d7174 100644 --- a/tests/ui/type-alias-impl-trait/non-defining-method.stderr +++ b/tests/ui/type-alias-impl-trait/non-defining-method.stderr @@ -1,4 +1,4 @@ -error: non-defining opaque type use in defining scope +error[E0792]: non-defining opaque type use in defining scope --> $DIR/non-defining-method.rs:16:17 | LL | fn foo() -> Self::Bar {} @@ -12,3 +12,4 @@ LL | type Bar = impl Sized; error: aborting due to previous error +For more information about this error, try `rustc --explain E0792`. From cb161e77ba7492524e17e077085f1fab7e0dd9b8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Jun 2023 09:51:28 +0000 Subject: [PATCH 09/13] Move some field extraction logic onto a method on `Node` --- compiler/rustc_hir/src/hir.rs | 23 +++++++++++++++++++++ compiler/rustc_ty_utils/src/opaque_types.rs | 10 +++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 70fc66947df99..5e5001bc8b450 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -3743,6 +3743,29 @@ impl<'hir> Node<'hir> { } } + /// Get the type for constants, assoc types, type aliases and statics. + pub fn ty(self) -> Option<&'hir Ty<'hir>> { + match self { + Node::Item(it) => match it.kind { + ItemKind::TyAlias(ty, _) | ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => { + Some(ty) + } + _ => None, + }, + Node::TraitItem(it) => match it.kind { + TraitItemKind::Const(ty, _) => Some(ty), + TraitItemKind::Type(_, ty) => ty, + _ => None, + }, + Node::ImplItem(it) => match it.kind { + ImplItemKind::Const(ty, _) => Some(ty), + ImplItemKind::Type(ty) => Some(ty), + _ => None, + }, + _ => None, + } + } + pub fn alias_ty(self) -> Option<&'hir Ty<'hir>> { match self { Node::Item(Item { kind: ItemKind::TyAlias(ty, ..), .. }) => Some(ty), diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index ab97406452e58..5fb5c4ad144a8 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -177,13 +177,9 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [ } } DefKind::AssocTy | DefKind::AssocConst => { - let span = match tcx.hir().get_by_def_id(item) { - rustc_hir::Node::ImplItem(it) => match it.kind { - rustc_hir::ImplItemKind::Const(ty, _) => ty.span, - rustc_hir::ImplItemKind::Type(ty) => ty.span, - other => span_bug!(tcx.def_span(item), "{other:#?}"), - }, - other => span_bug!(tcx.def_span(item), "{other:#?}"), + let span = match tcx.hir().get_by_def_id(item).ty() { + Some(ty) => ty.span, + _ => tcx.def_span(item), }; collector.visit_spanned(span, tcx.type_of(item).subst_identity()); } From 0af4a211be2c9cc345728449a2fe213f64c216b8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Jun 2023 09:55:01 +0000 Subject: [PATCH 10/13] Document what is going on in `opaque_types_defined_by` --- compiler/rustc_ty_utils/src/opaque_types.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 5fb5c4ad144a8..498194a7c10f6 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -168,14 +168,17 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [ DefKind::Fn | DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => { let mut collector = OpaqueTypeCollector::new(tcx, item); match kind { + // Walk over the signature of the function-like to find the opaques. DefKind::AssocFn | DefKind::Fn => { let ty_sig = tcx.fn_sig(item).subst_identity(); let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap(); + // Walk over the inputs and outputs manually in order to get good spans for them. collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output()); for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) { collector.visit_spanned(hir.span, ty.map_bound(|x| *x)); } } + // Walk over the type of the item to find opaques. DefKind::AssocTy | DefKind::AssocConst => { let span = match tcx.hir().get_by_def_id(item).ty() { Some(ty) => ty.span, From b0b4a0795949aee0df9bad292ba2951f8c6291ec Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Jun 2023 11:19:05 +0000 Subject: [PATCH 11/13] ICE on types that should not be defining opaque types --- compiler/rustc_ty_utils/src/opaque_types.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 498194a7c10f6..b8dfff0b8abf4 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -217,7 +217,9 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [ | DefKind::GlobalAsm | DefKind::Impl { .. } | DefKind::Closure - | DefKind::Generator => &[], + | DefKind::Generator => { + span_bug!(tcx.def_span(item), "{kind:?} is type checked as part of its parent") + } } } From b323f587fcd978deff5b922e60dcc4bcd208f2ab Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Jun 2023 12:42:59 +0000 Subject: [PATCH 12/13] Handle weak type aliases by immediately resolving them to their aliased type --- compiler/rustc_ty_utils/src/opaque_types.rs | 23 +++++++++++-------- ...s-impl-trait-declaration-too-subtle.stderr | 5 ---- .../unnameable_type.stderr | 5 ---- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index b8dfff0b8abf4..351c703cba128 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -107,6 +107,12 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { } } } + ty::Alias(ty::Weak, alias_ty) if alias_ty.def_id.is_local() => { + self.tcx + .type_of(alias_ty.def_id) + .subst(self.tcx, alias_ty.substs) + .visit_with(self)?; + } ty::Alias(ty::Projection, alias_ty) => { // This avoids having to do normalization of `Self::AssocTy` by only // supporting the case of a method defining opaque types from assoc types @@ -136,24 +142,23 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { ty::InternalSubsts::identity_for_item(self.tcx, parent), ); - if !check_substs_compatible(self.tcx, assoc, impl_substs) { + if check_substs_compatible(self.tcx, assoc, impl_substs) { + return self + .tcx + .type_of(assoc.def_id) + .subst(self.tcx, impl_substs) + .visit_with(self); + } else { self.tcx.sess.delay_span_bug( self.tcx.def_span(assoc.def_id), "item had incorrect substs", ); - return ControlFlow::Continue(()); } - - return self - .tcx - .type_of(assoc.def_id) - .subst(self.tcx, impl_substs) - .visit_with(self); } } } } - _ => {} + _ => trace!(kind=?t.kind()), } ControlFlow::Continue(()) } diff --git a/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr b/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr index 3e1981f096c77..fe765271bd2fe 100644 --- a/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr +++ b/tests/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr @@ -20,11 +20,6 @@ LL | fn eq(&self, _other: &(Foo, i32)) -> bool { | = note: expected signature `fn(&a::Bar, &(a::Bar, i32)) -> _` found signature `fn(&a::Bar, &(a::Foo, i32)) -> _` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:10:12 - | -LL | fn eq(&self, _other: &(Foo, i32)) -> bool { - | ^^ error: unconstrained opaque type --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:18:16 diff --git a/tests/ui/type-alias-impl-trait/unnameable_type.stderr b/tests/ui/type-alias-impl-trait/unnameable_type.stderr index c609ace919e5b..e9032433494a6 100644 --- a/tests/ui/type-alias-impl-trait/unnameable_type.stderr +++ b/tests/ui/type-alias-impl-trait/unnameable_type.stderr @@ -25,11 +25,6 @@ LL | fn dont_define_this(_private: Private) {} | ^^^^^^^ = note: expected signature `fn(Private)` found signature `fn(MyPrivate)` -note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/unnameable_type.rs:20:8 - | -LL | fn dont_define_this(_private: MyPrivate) {} - | ^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors From 27b386ad17720523ad7bc49e9cf481dd7f7d8da4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Jun 2023 17:58:14 +0000 Subject: [PATCH 13/13] Only walk the identity substituted version of struct fields --- compiler/rustc_ty_utils/src/opaque_types.rs | 23 +++++++++++++- ...n_behind_projection_behind_struct_field.rs | 28 +++++++++++++++++ ...hind_projection_behind_struct_field.stderr | 20 +++++++++++++ .../hidden_behind_struct_field.rs | 30 +++++++++++++++++++ .../hidden_behind_struct_field2.rs | 26 ++++++++++++++++ 5 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.rs create mode 100644 tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.stderr create mode 100644 tests/ui/type-alias-impl-trait/hidden_behind_struct_field.rs create mode 100644 tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs index 351c703cba128..29de8bf0e53f5 100644 --- a/compiler/rustc_ty_utils/src/opaque_types.rs +++ b/compiler/rustc_ty_utils/src/opaque_types.rs @@ -82,10 +82,12 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { // start seeing the error below. // Collect opaque types nested within the associated type bounds of this opaque type. + // We use identity substs here, because we already know that the opaque type uses + // only generic parameters, and thus substituting would not give us more information. for (pred, span) in self .tcx .explicit_item_bounds(alias_ty.def_id) - .subst_iter_copied(self.tcx, alias_ty.substs) + .subst_identity_iter_copied() { trace!(?pred); self.visit_spanned(span, pred); @@ -158,6 +160,25 @@ impl<'tcx> TypeVisitor> for OpaqueTypeCollector<'tcx> { } } } + ty::Adt(def, _) if def.did().is_local() => { + if !self.seen.insert(def.did().expect_local()) { + return ControlFlow::Continue(()); + } + for variant in def.variants().iter() { + for field in variant.fields.iter() { + // Don't use the `ty::Adt` substs, we either + // * found the opaque in the substs + // * will find the opaque in the unsubstituted fields + // The only other situation that can occur is that after substituting, + // some projection resolves to an opaque that we would have otherwise + // not found. While we could substitute and walk those, that would mean we + // would have to walk all substitutions of an Adt, which can quickly + // degenerate into looking at an exponential number of types. + let ty = self.tcx.type_of(field.did).subst_identity(); + self.visit_spanned(self.tcx.def_span(field.did), ty); + } + } + } _ => trace!(kind=?t.kind()), } ControlFlow::Continue(()) diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.rs b/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.rs new file mode 100644 index 0000000000000..eb19b49c7e213 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.rs @@ -0,0 +1,28 @@ +//! This test shows that a field type that is a projection that resolves to an opaque, +//! is not a defining use. While we could substitute the struct generics, that would +//! mean we would have to walk all substitutions of an `Foo`, which can quickly +//! degenerate into looking at an exponential number of types depending on the complexity +//! of a program. + +#![feature(impl_trait_in_assoc_type)] + +struct Bar; + +trait Trait: Sized { + type Assoc; + fn foo() -> Foo; +} + +impl Trait for Bar { + type Assoc = impl std::fmt::Debug; + fn foo() -> Foo { + Foo { field: () } + //~^ ERROR: mismatched types + } +} + +struct Foo { + field: ::Assoc, +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.stderr b/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.stderr new file mode 100644 index 0000000000000..648efd1cbfe7c --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_behind_projection_behind_struct_field.stderr @@ -0,0 +1,20 @@ +error[E0308]: mismatched types + --> $DIR/hidden_behind_projection_behind_struct_field.rs:19:22 + | +LL | type Assoc = impl std::fmt::Debug; + | -------------------- the expected opaque type +LL | fn foo() -> Foo { +LL | Foo { field: () } + | ^^ expected opaque type, found `()` + | + = note: expected opaque type `::Assoc` + found unit type `()` +note: this item must have the opaque type in its signature in order to be able to register hidden types + --> $DIR/hidden_behind_projection_behind_struct_field.rs:18:8 + | +LL | fn foo() -> Foo { + | ^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_struct_field.rs b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field.rs new file mode 100644 index 0000000000000..58778702de666 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field.rs @@ -0,0 +1,30 @@ +//! This test shows that the appearance of an opaque type +//! in the substs of a struct are enough to make it count +//! for making the function a defining use. It doesn't matter +//! if the opaque type is actually used in the field. + +#![feature(impl_trait_in_assoc_type)] +// check-pass + +use std::marker::PhantomData; + +struct Bar; + +trait Trait: Sized { + type Assoc; + fn foo() -> Foo; +} + +impl Trait for Bar { + type Assoc = impl std::fmt::Debug; + fn foo() -> Foo { + let foo: Foo<()> = Foo { field: PhantomData }; + foo + } +} + +struct Foo { + field: PhantomData, +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs new file mode 100644 index 0000000000000..e440dce5e514f --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_behind_struct_field2.rs @@ -0,0 +1,26 @@ +//! This test shows that we can even follow projections +//! into associated types of the same impl if they are +//! indirectly mentioned in a struct field. + +#![feature(impl_trait_in_assoc_type)] +// check-pass + +struct Bar; + +trait Trait: Sized { + type Assoc; + fn foo() -> Foo; +} + +impl Trait for Bar { + type Assoc = impl std::fmt::Debug; + fn foo() -> Foo { + Foo { field: () } + } +} + +struct Foo { + field: ::Assoc, +} + +fn main() {}