Temporarily emulate the (accidentally) omitted recursion during impl Trait check.

Note that the two previous visitors were omitting slightly different
recursive calls, so I need two flags to properly emulate them.

Author: pnkfelix

src/librustc/lint/builtin.rs

@@ -386,6 +386,12 @@ declare_lint! {
     "ambiguous associated items"
 }
 
+declare_lint! {
+    pub NESTED_IMPL_TRAIT,
+    Warn,
+    "nested occurrence of `impl Trait` type"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// that are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -457,6 +463,7 @@ impl LintPass for HardwiredLints {
             parser::ILL_FORMED_ATTRIBUTE_INPUT,
             DEPRECATED_IN_FUTURE,
             AMBIGUOUS_ASSOCIATED_ITEMS,
+            NESTED_IMPL_TRAIT,
         )
     }
 }
@@ -474,6 +481,7 @@ pub enum BuiltinLintDiagnostics {
     ElidedLifetimesInPaths(usize, Span, bool, Span, String),
     UnknownCrateTypes(Span, String, String),
     UnusedImports(String, Vec<(Span, String)>),
+    NestedImplTrait { outer_impl_trait_span: Span, inner_impl_trait_span: Span },
 }
 
 impl BuiltinLintDiagnostics {
@@ -564,6 +572,12 @@ impl BuiltinLintDiagnostics {
                     );
                 }
             }
+            BuiltinLintDiagnostics::NestedImplTrait {
+                outer_impl_trait_span, inner_impl_trait_span
+            } => {
+                db.span_label(outer_impl_trait_span, "outer `impl Trait`");
+                db.span_label(inner_impl_trait_span, "nested `impl Trait` here");
+            }
         }
     }
 }

src/librustc_lint/lib.rs

@@ -353,6 +353,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             reference: "issue #57593 <https://github.com/rust-lang/rust/issues/57593>",
             edition: None,
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(NESTED_IMPL_TRAIT),
+            reference: "issue #59014 <https://github.com/rust-lang/rust/issues/59014>",
+            edition: None,
+        },
         ]);
 
     // Register renamed and removed lints.

src/librustc_passes/ast_validation.rs

@@ -9,6 +9,7 @@
 use std::mem;
 use syntax::print::pprust;
 use rustc::lint;
+use rustc::lint::builtin::{BuiltinLintDiagnostics, NESTED_IMPL_TRAIT};
 use rustc::session::Session;
 use rustc_data_structures::fx::FxHashMap;
 use syntax::ast::*;
@@ -36,9 +37,32 @@ struct AstValidator<'a> {
     // Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
     // or `Foo::Bar<impl Trait>`
     is_impl_trait_banned: bool,
+
+    // rust-lang/rust#57979: the ban of nested `impl Trait` was buggy
+    // until sometime after PR #57730 landed: it would jump directly
+    // to walk_ty rather than visit_ty (or skip recurring entirely for
+    // impl trait in projections), and thus miss some cases. We track
+    // whether we should downgrade to a warning for short-term via
+    // these booleans.
+    warning_period_57979_nested_impl_trait: bool,
+    warning_period_57979_impl_trait_in_proj: bool,
 }
 
 impl<'a> AstValidator<'a> {
+    fn with_nested_impl_trait_warning<T>(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T {
+        let old = mem::replace(&mut self.warning_period_57979_nested_impl_trait, v);
+        let ret = f(self);
+        self.warning_period_57979_nested_impl_trait = old;
+        ret
+    }
+
+    fn with_impl_trait_in_proj_warning<T>(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T {
+        let old = mem::replace(&mut self.warning_period_57979_impl_trait_in_proj, v);
+        let ret = f(self);
+        self.warning_period_57979_impl_trait_in_proj = old;
+        ret
+    }
+
     fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
         let old = mem::replace(&mut self.is_impl_trait_banned, true);
         f(self);
@@ -406,22 +430,41 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             }
             TyKind::ImplTrait(_, ref bounds) => {
                 if self.is_impl_trait_banned {
-                    struct_span_err!(self.session, ty.span, E0667,
-                        "`impl Trait` is not allowed in path parameters").emit();
+                    if self.warning_period_57979_impl_trait_in_proj {
+                        self.session.buffer_lint(
+                            NESTED_IMPL_TRAIT, ty.id, ty.span,
+                            "`impl Trait` is not allowed in path parameters");
+                    } else {
+                        struct_span_err!(self.session, ty.span, E0667,
+                            "`impl Trait` is not allowed in path parameters").emit();
+                    }
                 }
 
                 if let Some(outer_impl_trait) = self.outer_impl_trait {
-                    struct_span_err!(self.session, ty.span, E0666,
-                                    "nested `impl Trait` is not allowed")
-                        .span_label(outer_impl_trait, "outer `impl Trait`")
-                        .span_label(ty.span, "nested `impl Trait` here")
-                        .emit();
-
+                    if self.warning_period_57979_nested_impl_trait {
+                        self.session.buffer_lint_with_diagnostic(
+                            NESTED_IMPL_TRAIT, ty.id, ty.span,
+                            "nested `impl Trait` is not allowed",
+                            BuiltinLintDiagnostics::NestedImplTrait {
+                                outer_impl_trait_span: outer_impl_trait,
+                                inner_impl_trait_span: ty.span,
+                            });
+                    } else {
+                        struct_span_err!(self.session, ty.span, E0666,
+                            "nested `impl Trait` is not allowed")
+                            .span_label(outer_impl_trait, "outer `impl Trait`")
+                            .span_label(ty.span, "nested `impl Trait` here")
+                            .emit();
+                    }
                 }
+
                 if !bounds.iter()
                           .any(|b| if let GenericBound::Trait(..) = *b { true } else { false }) {
                     self.err_handler().span_err(ty.span, "at least one trait must be specified");
                 }
+
+                self.with_impl_trait_in_proj_warning(true, |this| this.walk_ty(ty));
+                return;
             }
             _ => {}
         }
@@ -606,18 +649,23 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                         GenericArg::Const(..) => ParamKindOrd::Const,
                     }, arg.span(), None)
                 }), GenericPosition::Arg, generic_args.span());
-                // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
-                // are allowed to contain nested `impl Trait`.
-                self.with_impl_trait(None, |this| {
-                    walk_list!(this, visit_assoc_type_binding, &data.bindings);
+
+                self.with_nested_impl_trait_warning(true, |this| {
+                    // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
+                    // are allowed to contain nested `impl Trait`.
+                    this.with_impl_trait(None, |this| {
+                        walk_list!(this, visit_assoc_type_binding, &data.bindings);
+                    });
                 });
             }
             GenericArgs::Parenthesized(ref data) => {
                 walk_list!(self, visit_ty, &data.inputs);
                 if let Some(ref type_) = data.output {
-                    // `-> Foo` syntax is essentially an associated type binding,
-                    // so it is also allowed to contain nested `impl Trait`.
-                    self.with_impl_trait(None, |this| this.visit_ty(type_));
+                    self.with_nested_impl_trait_warning(true, |this| {
+                        // `-> Foo` syntax is essentially an associated type binding,
+                        // so it is also allowed to contain nested `impl Trait`.
+                        this.with_impl_trait(None, |this| this.visit_ty(type_));
+                    });
                 }
             }
         }
@@ -719,6 +767,8 @@ pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) {
         has_global_allocator: false,
         outer_impl_trait: None,
         is_impl_trait_banned: false,
+        warning_period_57979_nested_impl_trait: false,
+        warning_period_57979_impl_trait_in_proj: false,
     };
     visit::walk_crate(&mut validator, krate);