small TypeVisitor refactor

rust-lang · Dec 7, 2020 · e3e4870 · e3e4870
1 parent 0f6f2d6
commit e3e4870
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 36 deletions.
diff --git a/compiler/rustc_mir/src/interpret/util.rs b/compiler/rustc_mir/src/interpret/util.rs
@@ -13,20 +13,21 @@ where
         return Ok(());
     }
 
+    struct FoundParam;
     struct UsedParamsNeedSubstVisitor<'tcx> {
         tcx: TyCtxt<'tcx>,
     }
 
     impl<'tcx> TypeVisitor<'tcx> for UsedParamsNeedSubstVisitor<'tcx> {
-        type BreakTy = ();
+        type BreakTy = FoundParam;
 
         fn visit_const(&mut self, c: &'tcx ty::Const<'tcx>) -> ControlFlow<Self::BreakTy> {
             if !c.needs_subst() {
                 return ControlFlow::CONTINUE;
             }
 
             match c.val {
-                ty::ConstKind::Param(..) => ControlFlow::BREAK,
+                ty::ConstKind::Param(..) => ControlFlow::Break(FoundParam),
                 _ => c.super_visit_with(self),
             }
         }
@@ -37,7 +38,7 @@ where
             }
 
             match *ty.kind() {
-                ty::Param(_) => ControlFlow::BREAK,
+                ty::Param(_) => ControlFlow::Break(FoundParam),
                 ty::Closure(def_id, substs)
                 | ty::Generator(def_id, substs, ..)
                 | ty::FnDef(def_id, substs) => {
@@ -76,7 +77,7 @@ where
     }
 
     let mut vis = UsedParamsNeedSubstVisitor { tcx };
-    if ty.visit_with(&mut vis).is_break() {
+    if matches!(ty.visit_with(&mut vis), ControlFlow::Break(FoundParam)) {
         throw_inval!(TooGeneric);
     } else {
         Ok(())

diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs
@@ -462,39 +462,25 @@ pub(super) fn check_opaque<'tcx>(
 
 /// Checks that an opaque type does not use `Self` or `T::Foo` projections that would result
 /// in "inheriting lifetimes".
+#[instrument(skip(tcx, span))]
 pub(super) fn check_opaque_for_inheriting_lifetimes(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
     span: Span,
 ) {
     let item = tcx.hir().expect_item(tcx.hir().local_def_id_to_hir_id(def_id));
-    debug!(
-        "check_opaque_for_inheriting_lifetimes: def_id={:?} span={:?} item={:?}",
-        def_id, span, item
-    );
-
-    #[derive(Debug)]
-    struct ProhibitOpaqueVisitor<'tcx> {
-        opaque_identity_ty: Ty<'tcx>,
-        generics: &'tcx ty::Generics,
-    }
+    debug!(?item, ?span);
 
-    impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueVisitor<'tcx> {
-        type BreakTy = Option<Ty<'tcx>>;
-
-        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-            debug!("check_opaque_for_inheriting_lifetimes: (visit_ty) t={:?}", t);
-            if t != self.opaque_identity_ty && t.super_visit_with(self).is_break() {
-                return ControlFlow::Break(Some(t));
-            }
-            ControlFlow::CONTINUE
-        }
+    struct FoundParentLifetime;
+    struct FindParentLifetimeVisitor<'tcx>(&'tcx ty::Generics);
+    impl<'tcx> ty::fold::TypeVisitor<'tcx> for FindParentLifetimeVisitor<'tcx> {
+        type BreakTy = FoundParentLifetime;
 
         fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<Self::BreakTy> {
-            debug!("check_opaque_for_inheriting_lifetimes: (visit_region) r={:?}", r);
+            debug!("FindParentLifetimeVisitor: r={:?}", r);
             if let RegionKind::ReEarlyBound(ty::EarlyBoundRegion { index, .. }) = r {
-                if *index < self.generics.parent_count as u32 {
-                    return ControlFlow::Break(None);
+                if *index < self.0.parent_count as u32 {
+                    return ControlFlow::Break(FoundParentLifetime);
                 } else {
                     return ControlFlow::CONTINUE;
                 }
@@ -505,7 +491,7 @@ pub(super) fn check_opaque_for_inheriting_lifetimes(
 
         fn visit_const(&mut self, c: &'tcx ty::Const<'tcx>) -> ControlFlow<Self::BreakTy> {
             if let ty::ConstKind::Unevaluated(..) = c.val {
-                // FIXME(#72219) We currenctly don't detect lifetimes within substs
+                // FIXME(#72219) We currently don't detect lifetimes within substs
                 // which would violate this check. Even though the particular substitution is not used
                 // within the const, this should still be fixed.
                 return ControlFlow::CONTINUE;
@@ -514,6 +500,26 @@ pub(super) fn check_opaque_for_inheriting_lifetimes(
         }
     }
 
+    #[derive(Debug)]
+    struct ProhibitOpaqueVisitor<'tcx> {
+        opaque_identity_ty: Ty<'tcx>,
+        generics: &'tcx ty::Generics,
+    }
+
+    impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueVisitor<'tcx> {
+        type BreakTy = Ty<'tcx>;
+
+        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+            debug!("check_opaque_for_inheriting_lifetimes: (visit_ty) t={:?}", t);
+            if t == self.opaque_identity_ty {
+                ControlFlow::CONTINUE
+            } else {
+                t.super_visit_with(&mut FindParentLifetimeVisitor(self.generics))
+                    .map_break(|FoundParentLifetime| t)
+            }
+        }
+    }
+
     if let ItemKind::OpaqueTy(hir::OpaqueTy {
         origin: hir::OpaqueTyOrigin::AsyncFn | hir::OpaqueTyOrigin::FnReturn,
         ..
@@ -555,14 +561,12 @@ pub(super) fn check_opaque_for_inheriting_lifetimes(
 
             if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(span) {
                 if snippet == "Self" {
-                    if let Some(ty) = ty {
-                        err.span_suggestion(
-                            span,
-                            "consider spelling out the type instead",
-                            format!("{:?}", ty),
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
+                    err.span_suggestion(
+                        span,
+                        "consider spelling out the type instead",
+                        format!("{:?}", ty),
+                        Applicability::MaybeIncorrect,
+                    );
                 }
             }
             err.emit();

diff --git a/library/core/src/ops/control_flow.rs b/library/core/src/ops/control_flow.rs
@@ -56,6 +56,20 @@ impl<B, C> ControlFlow<B, C> {
             ControlFlow::Break(x) => Some(x),
         }
     }
+
+    /// Maps `ControlFlow<B, C>` to `ControlFlow<T, C>` by applying a function
+    /// to the break value in case it exists.
+    #[inline]
+    #[unstable(feature = "control_flow_enum", reason = "new API", issue = "75744")]
+    pub fn map_break<T, F>(self, f: F) -> ControlFlow<T, C>
+    where
+        F: FnOnce(B) -> T,
+    {
+        match self {
+            ControlFlow::Continue(x) => ControlFlow::Continue(x),
+            ControlFlow::Break(x) => ControlFlow::Break(f(x)),
+        }
+    }
 }
 
 impl<R: Try> ControlFlow<R, R::Ok> {