-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Require TAITs to appear in the signature of items that register a hidden type #107809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
73cfef3
0e5d7b8
86a360d
375dfa4
bd86dcb
d715644
733e444
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| use std::ops::ControlFlow; | ||
|
|
||
| use hir::def::DefKind; | ||
| use rustc_data_structures::fx::FxHashSet; | ||
| use rustc_errors::{Applicability, StashKey}; | ||
| use rustc_hir as hir; | ||
| use rustc_hir::def_id::{DefId, LocalDefId}; | ||
|
|
@@ -9,15 +13,15 @@ use rustc_middle::ty::print::with_forced_trimmed_paths; | |
| use rustc_middle::ty::subst::InternalSubsts; | ||
| use rustc_middle::ty::util::IntTypeExt; | ||
| use rustc_middle::ty::{ | ||
| self, ImplTraitInTraitData, IsSuggestable, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, | ||
| TypeVisitableExt, | ||
| self, ImplTraitInTraitData, IsSuggestable, Ty, TyCtxt, TypeFoldable, TypeFolder, | ||
| TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, | ||
| }; | ||
| use rustc_span::symbol::Ident; | ||
| use rustc_span::{Span, DUMMY_SP}; | ||
|
|
||
| use super::ItemCtxt; | ||
| use super::{bad_placeholder, is_suggestable_infer_ty}; | ||
| use crate::errors::UnconstrainedOpaqueType; | ||
| use crate::errors::{OpaqueTypeConstrainedButNotInSig, UnconstrainedOpaqueType}; | ||
|
|
||
| /// Computes the relevant generic parameter for a potential generic const argument. | ||
| /// | ||
|
|
@@ -635,6 +639,12 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T | |
| let concrete_opaque_types = &self.tcx.mir_borrowck(item_def_id).concrete_opaque_types; | ||
| debug!(?concrete_opaque_types); | ||
| if let Some(&concrete_type) = concrete_opaque_types.get(&self.def_id) { | ||
| if !may_define_opaque_type(self.tcx, item_def_id, self.def_id) { | ||
| self.tcx.sess.emit_err(OpaqueTypeConstrainedButNotInSig { | ||
| span: concrete_type.span, | ||
| item_span: self.tcx.def_span(item_def_id), | ||
| }); | ||
| } | ||
| debug!(?concrete_type, "found constraint"); | ||
| if let Some(prev) = &mut self.found { | ||
| if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() { | ||
|
|
@@ -743,6 +753,117 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T | |
| hidden.ty | ||
| } | ||
|
|
||
| #[instrument(skip(tcx), level = "trace", ret)] | ||
| fn may_define_opaque_type<'tcx>( | ||
| tcx: TyCtxt<'tcx>, | ||
| def_id: LocalDefId, | ||
| opaque_def_id: LocalDefId, | ||
| ) -> bool { | ||
| if tcx.is_descendant_of(opaque_def_id.to_def_id(), def_id.to_def_id()) { | ||
| // If the opaque type is defined in the body of a function, that function | ||
| // may constrain the opaque type since it can't mention it in bounds. | ||
| return true; | ||
| } | ||
|
|
||
| if tcx.is_closure(def_id.to_def_id()) { | ||
| return may_define_opaque_type(tcx, tcx.local_parent(def_id), opaque_def_id); | ||
| } | ||
|
|
||
| let param_env = tcx.param_env(def_id); | ||
|
|
||
| trace!(parent = ?tcx.parent(opaque_def_id.to_def_id())); | ||
| fn has_tait<'tcx>( | ||
| val: impl TypeVisitable<TyCtxt<'tcx>>, | ||
| opaque_def_id: LocalDefId, | ||
| tcx: TyCtxt<'tcx>, | ||
| param_env: ty::ParamEnv<'tcx>, | ||
| ) -> bool { | ||
| struct Visitor<'tcx> { | ||
| opaque_def_id: DefId, | ||
| tcx: TyCtxt<'tcx>, | ||
| seen: FxHashSet<Ty<'tcx>>, | ||
| param_env: ty::ParamEnv<'tcx>, | ||
| } | ||
| impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Visitor<'tcx> { | ||
| type BreakTy = (); | ||
|
|
||
| #[instrument(skip(self), level = "trace", ret)] | ||
| fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> { | ||
| // Erase all lifetimes, they can't affect anything, but recursion | ||
| // may cause late bound regions to differ, because the type visitor | ||
| // can't erase them in `visit_binder`. | ||
| let t = t.fold_with(&mut ty::fold::BottomUpFolder { | ||
| tcx: self.tcx, | ||
| ct_op: |c| c, | ||
| ty_op: |t| t, | ||
| lt_op: |_| self.tcx.lifetimes.re_erased, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't erase bound regions here, only escaping bound regions 🤔
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could implement |
||
| }); | ||
| if !self.seen.insert(t) { | ||
| return ControlFlow::Continue(()); | ||
| } | ||
| match t.kind() { | ||
| ty::Alias(ty::Opaque, alias) => { | ||
| if alias.def_id == self.opaque_def_id { | ||
| return ControlFlow::Break(()); | ||
| } | ||
| for (pred, _span) in self | ||
| .tcx | ||
| .bound_explicit_item_bounds(alias.def_id) | ||
| .subst_iter_copied(self.tcx, alias.substs) | ||
| { | ||
| pred.visit_with(self)?; | ||
| } | ||
| } | ||
| ty::Alias(ty::Projection, _) => { | ||
| if let Ok(proj) = self.tcx.try_normalize_erasing_regions(self.param_env, t) | ||
| { | ||
| proj.visit_with(self)?; | ||
| } | ||
| } | ||
| // Types that have opaque type fields must get walked manually, they | ||
| // would not be seen by the type visitor otherwise. | ||
| ty::Adt(adt_def, substs) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the type can be contained in an also, this can be |
||
| for variant in adt_def.variants() { | ||
| for field in &variant.fields { | ||
| field.ty(self.tcx, substs).visit_with(self)?; | ||
| } | ||
| } | ||
| } | ||
| _ => (), | ||
| } | ||
| t.super_visit_with(self) | ||
| } | ||
| } | ||
| val.visit_with(&mut Visitor { | ||
| opaque_def_id: opaque_def_id.to_def_id(), | ||
| tcx, | ||
| seen: Default::default(), | ||
| param_env, | ||
| }) | ||
| .is_break() | ||
| } | ||
| let tait_in_fn_sig = match tcx.def_kind(def_id) { | ||
| DefKind::AssocFn | DefKind::Fn => { | ||
| has_tait(tcx.fn_sig(def_id.to_def_id()).skip_binder(), opaque_def_id, tcx, param_env) | ||
| } | ||
| // Opaque types in types of contsts | ||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| DefKind::Static(_) | DefKind::Const | DefKind::AssocConst => { | ||
| has_tait(tcx.type_of(def_id.to_def_id()).skip_binder(), opaque_def_id, tcx, param_env) | ||
| } | ||
| // Nested opaque types | ||
| DefKind::OpaqueTy => has_tait( | ||
| tcx.bound_explicit_item_bounds(def_id.to_def_id()).skip_binder(), | ||
| opaque_def_id, | ||
| tcx, | ||
| param_env, | ||
| ), | ||
| _ => false, | ||
| }; | ||
| trace!(?tait_in_fn_sig); | ||
| tait_in_fn_sig | ||
| || has_tait(tcx.predicates_of(def_id.to_def_id()).predicates, opaque_def_id, tcx, param_env) | ||
| } | ||
|
|
||
| fn find_opaque_ty_constraints_for_rpit( | ||
| tcx: TyCtxt<'_>, | ||
| def_id: LocalDefId, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ mod writeback; | |
|
|
||
| pub use fn_ctxt::FnCtxt; | ||
| pub use inherited::Inherited; | ||
| use rustc_infer::infer::DefiningAnchor; | ||
|
|
||
| use crate::check::check_fn; | ||
| use crate::coercion::DynamicCoerceMany; | ||
|
|
@@ -212,11 +213,21 @@ fn typeck_with_fallback<'tcx>( | |
| } else { | ||
| param_env | ||
| }; | ||
| let inh = Inherited::new(tcx, def_id); | ||
| let fn_sig_infer = fn_sig.map_or(false, |fn_sig| { | ||
| rustc_hir_analysis::collect::get_infer_ret_ty(&fn_sig.decl.output).is_some() | ||
| }); | ||
|
|
||
| let mk_defining_use_anchor = |def_id| { | ||
| // In case we are inferring the return signature (via `_` types), ignore defining use | ||
| // rules, as we'll error out anyway. This helps improve diagnostics, which otherwise | ||
| // may just see `ty::Error` instead of `ty::Alias(Opaque, _)` and not produce better errors. | ||
| if fn_sig_infer { DefiningAnchor::Bubble } else { DefiningAnchor::Bind(def_id) } | ||
|
||
| }; | ||
| let inh = Inherited::new(tcx, def_id, mk_defining_use_anchor); | ||
| let mut fcx = FnCtxt::new(&inh, param_env, def_id); | ||
|
|
||
| if let Some(hir::FnSig { header, decl, .. }) = fn_sig { | ||
| let fn_sig = if rustc_hir_analysis::collect::get_infer_ret_ty(&decl.output).is_some() { | ||
| let fn_sig = if fn_sig_infer { | ||
| fcx.astconv().ty_of_fn(id, header.unsafety, header.abi, decl, None, None) | ||
| } else { | ||
| tcx.fn_sig(def_id).subst_identity() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this only apply for closures, or also for inline consts? IOW, should we check
is_typeck_childhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test for the ban on closure signatures? I saw tests/ui/type-alias-impl-trait/issue-63263-closure-return.rs is changed to accommodate it, but I missed the "fail" test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right on all accounts. There were no tests, and there were especially none for inline consts. I added them and fixed it so that inline consts actually work at all.