Require any function with a tait in its signature to actually constrain a hidden type

Author: oli-obk

compiler/rustc_data_structures/src/obligation_forest/mod.rs

@@ -146,8 +146,6 @@ pub enum ProcessResult<O, E> {
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
 struct ObligationTreeId(usize);
 
-type ObligationTreeIdGenerator = impl Iterator<Item = ObligationTreeId>;
-
 pub struct ObligationForest<O: ForestObligation> {
     /// The list of obligations. In between calls to [Self::process_obligations],
     /// this list only contains nodes in the `Pending` or `Waiting` state.
@@ -310,18 +308,25 @@ pub struct Error<O, E> {
     pub backtrace: Vec<O>,
 }
 
-impl<O: ForestObligation> ObligationForest<O> {
-    pub fn new() -> ObligationForest<O> {
-        ObligationForest {
-            nodes: vec![],
-            done_cache: Default::default(),
-            active_cache: Default::default(),
-            reused_node_vec: vec![],
-            obligation_tree_id_generator: (0..).map(ObligationTreeId),
-            error_cache: Default::default(),
+mod helper {
+    use super::*;
+    pub type ObligationTreeIdGenerator = impl Iterator<Item = ObligationTreeId>;
+    impl<O: ForestObligation> ObligationForest<O> {
+        pub fn new() -> ObligationForest<O> {
+            ObligationForest {
+                nodes: vec![],
+                done_cache: Default::default(),
+                active_cache: Default::default(),
+                reused_node_vec: vec![],
+                obligation_tree_id_generator: (0..).map(ObligationTreeId),
+                error_cache: Default::default(),
+            }
         }
     }
+}
+use helper::*;
 
+impl<O: ForestObligation> ObligationForest<O> {
     /// Returns the total number of nodes in the forest that have not
     /// yet been fully resolved.
     pub fn len(&self) -> usize {

compiler/rustc_hir_analysis/messages.ftl

@@ -312,6 +312,10 @@ hir_analysis_substs_on_overridden_impl = could not resolve substs on overridden
 hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
     .note = this item must mention the opaque type in its signature in order to be able to register hidden types
 
+hir_analysis_tait_forward_compat2 = item does not constrain `{$opaque_type}`, but has it in its signature
+    .note = consider moving the opaque type's declaration and defining uses into a separate module
+    .opaque = this opaque type is in the signature
+
 hir_analysis_target_feature_on_main = `main` function is not allowed to have `#[target_feature]`
 
 hir_analysis_too_large_static = extern static is too large for the current architecture

compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs

@@ -6,7 +6,7 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
 use rustc_span::DUMMY_SP;
 
-use crate::errors::{TaitForwardCompat, UnconstrainedOpaqueType};
+use crate::errors::{TaitForwardCompat, TaitForwardCompat2, UnconstrainedOpaqueType};
 
 /// Checks "defining uses" of opaque `impl Trait` types to ensure that they meet the restrictions
 /// laid for "higher-order pattern unification".
@@ -138,13 +138,17 @@ impl TaitConstraintLocator<'_> {
             return;
         }
 
+        let opaques = self.tcx.opaque_types_defined_by(item_def_id);
+        let opaque_type_must_be_defined = opaques.in_signature.contains(&self.def_id);
+        let opaque_type_may_be_defined = opaques.in_body.contains(&self.def_id);
+
         let mut constrained = false;
         for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
             if opaque_type_key.def_id != self.def_id {
                 continue;
             }
             constrained = true;
-            if !self.tcx.opaque_types_defined_by(item_def_id).contains(&self.def_id) {
+            if !opaque_type_must_be_defined && !opaque_type_may_be_defined {
                 self.tcx.sess.emit_err(TaitForwardCompat {
                     span: hidden_type.span,
                     item_span: self
@@ -166,6 +170,16 @@ impl TaitConstraintLocator<'_> {
 
         if !constrained {
             debug!("no constraints in typeck results");
+            if opaque_type_must_be_defined {
+                self.tcx.sess.emit_err(TaitForwardCompat2 {
+                    span: self
+                        .tcx
+                        .def_ident_span(item_def_id)
+                        .unwrap_or_else(|| self.tcx.def_span(item_def_id)),
+                    opaque_type_span: self.tcx.def_span(self.def_id),
+                    opaque_type: self.tcx.def_path_str(self.def_id),
+                });
+            }
             return;
         };
 

compiler/rustc_hir_analysis/src/errors.rs

@@ -194,6 +194,17 @@ pub struct TaitForwardCompat {
     pub item_span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(hir_analysis_tait_forward_compat2)]
+#[note]
+pub struct TaitForwardCompat2 {
+    #[primary_span]
+    pub span: Span,
+    #[note(hir_analysis_opaque)]
+    pub opaque_type_span: Span,
+    pub opaque_type: String,
+}
+
 pub struct MissingTypeParams {
     pub span: Span,
     pub def_span: Span,

compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs

@@ -289,9 +289,10 @@ impl<T> Trait<T> for X {
                             ) =>
                     {
                         if tcx.is_type_alias_impl_trait(alias.def_id) {
-                            if !tcx
+                            let opaques = tcx
                                 .opaque_types_defined_by(body_owner_def_id.expect_local())
-                                .contains(&alias.def_id.expect_local())
+                                ;
+                            if !opaques.in_body.contains(&alias.def_id.expect_local()) && !opaques.in_signature.contains(&alias.def_id.expect_local())
                             {
                                 let sp = tcx
                                     .def_ident_span(body_owner_def_id)

compiler/rustc_infer/src/infer/opaque_types.rs

@@ -387,7 +387,12 @@ impl<'tcx> InferCtxt<'tcx> {
             // Named `type Foo = impl Bar;`
             hir::OpaqueTyOrigin::TyAlias { in_assoc_ty } => {
                 if in_assoc_ty {
-                    self.tcx.opaque_types_defined_by(parent_def_id).contains(&def_id)
+                    self.tcx.opaque_types_defined_by(parent_def_id).in_body.contains(&def_id)
+                        || self
+                            .tcx
+                            .opaque_types_defined_by(parent_def_id)
+                            .in_signature
+                            .contains(&def_id)
                 } else {
                     may_define_opaque_type(self.tcx, parent_def_id, opaque_hir_id)
                 }

compiler/rustc_middle/src/mir/terminator.rs

@@ -284,7 +284,6 @@ pub struct Terminator<'tcx> {
     pub kind: TerminatorKind<'tcx>,
 }
 
-pub type Successors<'a> = impl DoubleEndedIterator<Item = BasicBlock> + 'a;
 pub type SuccessorsMut<'a> =
     iter::Chain<std::option::IntoIter<&'a mut BasicBlock>, slice::IterMut<'a, BasicBlock>>;
 
@@ -310,47 +309,57 @@ impl<'tcx> TerminatorKind<'tcx> {
     pub fn if_(cond: Operand<'tcx>, t: BasicBlock, f: BasicBlock) -> TerminatorKind<'tcx> {
         TerminatorKind::SwitchInt { discr: cond, targets: SwitchTargets::static_if(0, f, t) }
     }
+}
 
-    pub fn successors(&self) -> Successors<'_> {
-        use self::TerminatorKind::*;
-        match *self {
-            Call { target: Some(t), unwind: UnwindAction::Cleanup(ref u), .. }
-            | Yield { resume: t, drop: Some(ref u), .. }
-            | Drop { target: t, unwind: UnwindAction::Cleanup(ref u), .. }
-            | Assert { target: t, unwind: UnwindAction::Cleanup(ref u), .. }
-            | FalseUnwind { real_target: t, unwind: UnwindAction::Cleanup(ref u) }
-            | InlineAsm { destination: Some(t), unwind: UnwindAction::Cleanup(ref u), .. } => {
-                Some(t).into_iter().chain(slice::from_ref(u).into_iter().copied())
-            }
-            Goto { target: t }
-            | Call { target: None, unwind: UnwindAction::Cleanup(t), .. }
-            | Call { target: Some(t), unwind: _, .. }
-            | Yield { resume: t, drop: None, .. }
-            | Drop { target: t, unwind: _, .. }
-            | Assert { target: t, unwind: _, .. }
-            | FalseUnwind { real_target: t, unwind: _ }
-            | InlineAsm { destination: None, unwind: UnwindAction::Cleanup(t), .. }
-            | InlineAsm { destination: Some(t), unwind: _, .. } => {
-                Some(t).into_iter().chain((&[]).into_iter().copied())
-            }
-            UnwindResume
-            | UnwindTerminate(_)
-            | CoroutineDrop
-            | Return
-            | Unreachable
-            | Call { target: None, unwind: _, .. }
-            | InlineAsm { destination: None, unwind: _, .. } => {
-                None.into_iter().chain((&[]).into_iter().copied())
-            }
-            SwitchInt { ref targets, .. } => {
-                None.into_iter().chain(targets.targets.iter().copied())
+pub use helper::*;
+
+mod helper {
+    use super::*;
+    pub type Successors<'a> = impl DoubleEndedIterator<Item = BasicBlock> + 'a;
+    impl<'tcx> TerminatorKind<'tcx> {
+        pub fn successors(&self) -> Successors<'_> {
+            use self::TerminatorKind::*;
+            match *self {
+                Call { target: Some(t), unwind: UnwindAction::Cleanup(ref u), .. }
+                | Yield { resume: t, drop: Some(ref u), .. }
+                | Drop { target: t, unwind: UnwindAction::Cleanup(ref u), .. }
+                | Assert { target: t, unwind: UnwindAction::Cleanup(ref u), .. }
+                | FalseUnwind { real_target: t, unwind: UnwindAction::Cleanup(ref u) }
+                | InlineAsm {
+                    destination: Some(t), unwind: UnwindAction::Cleanup(ref u), ..
+                } => Some(t).into_iter().chain(slice::from_ref(u).into_iter().copied()),
+                Goto { target: t }
+                | Call { target: None, unwind: UnwindAction::Cleanup(t), .. }
+                | Call { target: Some(t), unwind: _, .. }
+                | Yield { resume: t, drop: None, .. }
+                | Drop { target: t, unwind: _, .. }
+                | Assert { target: t, unwind: _, .. }
+                | FalseUnwind { real_target: t, unwind: _ }
+                | InlineAsm { destination: None, unwind: UnwindAction::Cleanup(t), .. }
+                | InlineAsm { destination: Some(t), unwind: _, .. } => {
+                    Some(t).into_iter().chain((&[]).into_iter().copied())
+                }
+                UnwindResume
+                | UnwindTerminate(_)
+                | CoroutineDrop
+                | Return
+                | Unreachable
+                | Call { target: None, unwind: _, .. }
+                | InlineAsm { destination: None, unwind: _, .. } => {
+                    None.into_iter().chain((&[]).into_iter().copied())
+                }
+                SwitchInt { ref targets, .. } => {
+                    None.into_iter().chain(targets.targets.iter().copied())
+                }
+                FalseEdge { real_target, ref imaginary_target } => Some(real_target)
+                    .into_iter()
+                    .chain(slice::from_ref(imaginary_target).into_iter().copied()),
             }
-            FalseEdge { real_target, ref imaginary_target } => Some(real_target)
-                .into_iter()
-                .chain(slice::from_ref(imaginary_target).into_iter().copied()),
         }
     }
+}
 
+impl<'tcx> TerminatorKind<'tcx> {
     pub fn successors_mut(&mut self) -> SuccessorsMut<'_> {
         use self::TerminatorKind::*;
         match *self {

compiler/rustc_middle/src/query/erase.rs

@@ -158,6 +158,10 @@ impl EraseType for Option<mir::DestructuredConstant<'_>> {
     type Result = [u8; size_of::<Option<mir::DestructuredConstant<'static>>>()];
 }
 
+impl EraseType for ty::OpaqueTypes<'_> {
+    type Result = [u8; size_of::<ty::OpaqueTypes<'static>>()];
+}
+
 impl EraseType for Option<ty::EarlyBinder<ty::TraitRef<'_>>> {
     type Result = [u8; size_of::<Option<ty::EarlyBinder<ty::TraitRef<'static>>>>()];
 }

compiler/rustc_middle/src/query/mod.rs

@@ -341,7 +341,7 @@ rustc_queries! {
 
     query opaque_types_defined_by(
         key: LocalDefId
-    ) -> &'tcx [LocalDefId] {
+    ) -> ty::OpaqueTypes<'tcx> {
         desc {
             |tcx| "computing the opaque types defined by `{}`",
             tcx.def_path_str(key.to_def_id())

compiler/rustc_middle/src/ty/mod.rs

@@ -30,6 +30,7 @@ pub use adt::*;
 pub use assoc::*;
 pub use generic_args::*;
 pub use generics::*;
+pub use opaque_types::OpaqueTypes;
 use rustc_ast as ast;
 use rustc_ast::node_id::NodeMap;
 use rustc_attr as attr;

compiler/rustc_middle/src/ty/opaque_types.rs

@@ -3,7 +3,7 @@ use crate::ty::fold::{TypeFolder, TypeSuperFoldable};
 use crate::ty::{self, Ty, TyCtxt, TypeFoldable};
 use crate::ty::{GenericArg, GenericArgKind};
 use rustc_data_structures::fx::FxHashMap;
-use rustc_span::def_id::DefId;
+use rustc_span::def_id::{DefId, LocalDefId};
 use rustc_span::Span;
 
 /// Converts generic params of a TypeFoldable from one
@@ -223,3 +223,13 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReverseMapper<'tcx> {
         }
     }
 }
+
+/// The opaque types defined by an item.
+#[derive(Copy, Clone, Debug, HashStable)]
+pub struct OpaqueTypes<'tcx> {
+    /// Opaque types in the signature *must* be defined by the item itself.
+    pub in_signature: &'tcx [LocalDefId],
+    /// Opaque types declared in the body are not required to be defined by
+    /// the item itself, they could be defined by other nested items.
+    pub in_body: &'tcx [LocalDefId],
+}

compiler/rustc_ty_utils/src/opaque_types.rs

@@ -118,7 +118,8 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
                 let id = id.owner_id.def_id;
                 if let DefKind::TyAlias = self.collector.tcx.def_kind(id) {
                     let items = self.collector.tcx.opaque_types_defined_by(id);
-                    self.collector.opaques.extend(items);
+                    assert_eq!(items.in_body, []);
+                    self.collector.opaques.extend(items.in_signature);
                 }
             }
             #[instrument(level = "trace", skip(self))]
@@ -269,38 +270,39 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
     }
 }
 
-fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [LocalDefId] {
+fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> ty::OpaqueTypes<'tcx> {
     let kind = tcx.def_kind(item);
     trace!(?kind);
-    let mut collector = OpaqueTypeCollector::new(tcx, item);
+    let mut in_signature = OpaqueTypeCollector::new(tcx, item);
+    let mut in_body = 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).instantiate_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());
+            in_signature.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));
+                in_signature.visit_spanned(hir.span, ty.map_bound(|x| *x));
             }
-            collector.collect_body_and_predicate_taits();
+            in_body.collect_body_and_predicate_taits();
         }
         // Walk over the type of the item to find opaques.
         DefKind::Static(_) | DefKind::Const | DefKind::AssocConst | DefKind::AnonConst => {
             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).instantiate_identity());
-            collector.collect_body_and_predicate_taits();
+            in_signature.visit_spanned(span, tcx.type_of(item).instantiate_identity());
+            in_body.collect_body_and_predicate_taits();
         }
         // We're also doing this for `AssocTy` for the wf checks in `check_opaque_meets_bounds`
         DefKind::TyAlias | DefKind::AssocTy => {
-            tcx.type_of(item).instantiate_identity().visit_with(&mut collector);
+            tcx.type_of(item).instantiate_identity().visit_with(&mut in_signature);
         }
         DefKind::OpaqueTy => {
             for (pred, span) in tcx.explicit_item_bounds(item).instantiate_identity_iter_copied() {
-                collector.visit_spanned(span, pred);
+                in_signature.visit_spanned(span, pred);
             }
         }
         DefKind::Mod
@@ -327,7 +329,10 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [
             return tcx.opaque_types_defined_by(tcx.local_parent(item));
         }
     }
-    tcx.arena.alloc_from_iter(collector.opaques)
+    ty::OpaqueTypes {
+        in_signature: tcx.arena.alloc_from_iter(in_signature.opaques),
+        in_body: tcx.arena.alloc_from_iter(in_body.opaques),
+    }
 }
 
 pub(super) fn provide(providers: &mut Providers) {