Rollup merge of rust-lang#48084 - cramertj:impl-trait-errors, r=nikomatsakis

Manishearth · Manishearth · commit a79e5e210e5e · 2018-02-24T12:47:58.000-08:00
Error on nested impl Trait and path projections from impl Trait cc rust-lang#34511 r? @nikomatsakis
diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
@@ -419,6 +419,141 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     }
 }
 
+// Bans nested `impl Trait`, e.g. `impl Into<impl Debug>`.
+// Nested `impl Trait` _is_ allowed in associated type position,
+// e.g `impl Iterator<Item=impl Debug>`
+struct NestedImplTraitVisitor<'a> {
+    session: &'a Session,
+    outer_impl_trait: Option<Span>,
+}
+
+impl<'a> NestedImplTraitVisitor<'a> {
+    fn with_impl_trait<F>(&mut self, outer_impl_trait: Option<Span>, f: F)
+        where F: FnOnce(&mut NestedImplTraitVisitor<'a>)
+    {
+        let old_outer_impl_trait = self.outer_impl_trait;
+        self.outer_impl_trait = outer_impl_trait;
+        f(self);
+        self.outer_impl_trait = old_outer_impl_trait;
+    }
+}
+
+
+impl<'a> Visitor<'a> for NestedImplTraitVisitor<'a> {
+    fn visit_ty(&mut self, t: &'a Ty) {
+        if let TyKind::ImplTrait(_) = t.node {
+            if let Some(outer_impl_trait) = self.outer_impl_trait {
+                struct_span_err!(self.session, t.span, E0666,
+                                 "nested `impl Trait` is not allowed")
+                    .span_label(outer_impl_trait, "outer `impl Trait`")
+                    .span_label(t.span, "nested `impl Trait` here")
+                    .emit();
+
+            }
+            self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t));
+        } else {
+            visit::walk_ty(self, t);
+        }
+    }
+    fn visit_path_parameters(&mut self, _: Span, path_parameters: &'a PathParameters) {
+        match *path_parameters {
+            PathParameters::AngleBracketed(ref params) => {
+                for type_ in &params.types {
+                    self.visit_ty(type_);
+                }
+                for type_binding in &params.bindings {
+                    // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
+                    // are allowed to contain nested `impl Trait`.
+                    self.with_impl_trait(None, |this| visit::walk_ty(this, &type_binding.ty));
+                }
+            }
+            PathParameters::Parenthesized(ref params) => {
+                for type_ in &params.inputs {
+                    self.visit_ty(type_);
+                }
+                if let Some(ref type_) = params.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| visit::walk_ty(this, type_));
+                }
+            }
+        }
+    }
+}
+
+// Bans `impl Trait` in path projections like `<impl Iterator>::Item` or `Foo::Bar<impl Trait>`.
+struct ImplTraitProjectionVisitor<'a> {
+    session: &'a Session,
+    is_banned: bool,
+}
+
+impl<'a> ImplTraitProjectionVisitor<'a> {
+    fn with_ban<F>(&mut self, f: F)
+        where F: FnOnce(&mut ImplTraitProjectionVisitor<'a>)
+    {
+        let old_is_banned = self.is_banned;
+        self.is_banned = true;
+        f(self);
+        self.is_banned = old_is_banned;
+    }
+}
+
+impl<'a> Visitor<'a> for ImplTraitProjectionVisitor<'a> {
+    fn visit_ty(&mut self, t: &'a Ty) {
+        match t.node {
+            TyKind::ImplTrait(_) => {
+                if self.is_banned {
+                    struct_span_err!(self.session, t.span, E0667,
+                                 "`impl Trait` is not allowed in path parameters")
+                        .emit();
+                }
+            }
+            TyKind::Path(ref qself, ref path) => {
+                // We allow these:
+                //  - `Option<impl Trait>`
+                //  - `option::Option<impl Trait>`
+                //  - `option::Option<T>::Foo<impl Trait>
+                //
+                // But not these:
+                //  - `<impl Trait>::Foo`
+                //  - `option::Option<impl Trait>::Foo`.
+                //
+                // To implement this, we disallow `impl Trait` from `qself`
+                // (for cases like `<impl Trait>::Foo>`)
+                // but we allow `impl Trait` in `PathParameters`
+                // iff there are no more PathSegments.
+                if let Some(ref qself) = *qself {
+                    // `impl Trait` in `qself` is always illegal
+                    self.with_ban(|this| this.visit_ty(&qself.ty));
+                }
+
+                for (i, segment) in path.segments.iter().enumerate() {
+                    // Allow `impl Trait` iff we're on the final path segment
+                    if i == (path.segments.len() - 1) {
+                        visit::walk_path_segment(self, path.span, segment);
+                    } else {
+                        self.with_ban(|this|
+                            visit::walk_path_segment(this, path.span, segment));
+                    }
+                }
+            }
+            _ => visit::walk_ty(self, t),
+        }
+    }
+}
+
 pub fn check_crate(session: &Session, krate: &Crate) {
+    visit::walk_crate(
+        &mut NestedImplTraitVisitor {
+            session,
+            outer_impl_trait: None,
+        }, krate);
+
+    visit::walk_crate(
+        &mut ImplTraitProjectionVisitor {
+            session,
+            is_banned: false,
+        }, krate);
+
     visit::walk_crate(&mut AstValidator { session: session }, krate)
 }
diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs
@@ -304,4 +304,6 @@ register_diagnostics! {
     E0567, // auto traits can not have generic parameters
     E0568, // auto traits can not have super traits
     E0642, // patterns aren't allowed in methods without bodies
+    E0666, // nested `impl Trait` is illegal
+    E0667, // `impl Trait` in projections
 }
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
@@ -432,9 +432,6 @@ declare_features! (
     // `foo.rs` as an alternative to `foo/mod.rs`
     (active, non_modrs_mods, "1.24.0", Some(44660)),
 
-    // Nested `impl Trait`
-    (active, nested_impl_trait, "1.24.0", Some(34511)),
-
     // Termination trait in main (RFC 1937)
     (active, termination_trait, "1.24.0", Some(43301)),
 
@@ -1352,73 +1349,8 @@ fn contains_novel_literal(item: &ast::MetaItem) -> bool {
     }
 }
 
-// Bans nested `impl Trait`, e.g. `impl Into<impl Debug>`.
-// Nested `impl Trait` _is_ allowed in associated type position,
-// e.g `impl Iterator<Item=impl Debug>`
-struct NestedImplTraitVisitor<'a> {
-    context: &'a Context<'a>,
-    is_in_impl_trait: bool,
-}
-
-impl<'a> NestedImplTraitVisitor<'a> {
-    fn with_impl_trait<F>(&mut self, is_in_impl_trait: bool, f: F)
-        where F: FnOnce(&mut NestedImplTraitVisitor<'a>)
-    {
-        let old_is_in_impl_trait = self.is_in_impl_trait;
-        self.is_in_impl_trait = is_in_impl_trait;
-        f(self);
-        self.is_in_impl_trait = old_is_in_impl_trait;
-    }
-}
-
-
-impl<'a> Visitor<'a> for NestedImplTraitVisitor<'a> {
-    fn visit_ty(&mut self, t: &'a ast::Ty) {
-        if let ast::TyKind::ImplTrait(_) = t.node {
-            if self.is_in_impl_trait {
-                gate_feature_post!(&self, nested_impl_trait, t.span,
-                    "nested `impl Trait` is experimental"
-                );
-            }
-            self.with_impl_trait(true, |this| visit::walk_ty(this, t));
-        } else {
-            visit::walk_ty(self, t);
-        }
-    }
-    fn visit_path_parameters(&mut self, _: Span, path_parameters: &'a ast::PathParameters) {
-        match *path_parameters {
-            ast::PathParameters::AngleBracketed(ref params) => {
-                for type_ in &params.types {
-                    self.visit_ty(type_);
-                }
-                for type_binding in &params.bindings {
-                    // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
-                    // are allowed to contain nested `impl Trait`.
-                    self.with_impl_trait(false, |this| visit::walk_ty(this, &type_binding.ty));
-                }
-            }
-            ast::PathParameters::Parenthesized(ref params) => {
-                for type_ in &params.inputs {
-                    self.visit_ty(type_);
-                }
-                if let Some(ref type_) = params.output {
-                    // `-> Foo` syntax is essentially an associated type binding,
-                    // so it is also allowed to contain nested `impl Trait`.
-                    self.with_impl_trait(false, |this| visit::walk_ty(this, type_));
-                }
-            }
-        }
-    }
-}
-
 impl<'a> PostExpansionVisitor<'a> {
-    fn whole_crate_feature_gates(&mut self, krate: &ast::Crate) {
-        visit::walk_crate(
-            &mut NestedImplTraitVisitor {
-                context: self.context,
-                is_in_impl_trait: false,
-            }, krate);
-
+    fn whole_crate_feature_gates(&mut self, _krate: &ast::Crate) {
         for &(ident, span) in &*self.context.parse_sess.non_modrs_mods.borrow() {
             if !span.allows_unstable() {
                 let cx = &self.context;
diff --git a/src/test/compile-fail/impl-trait/where-allowed.rs b/src/test/compile-fail/impl-trait/where-allowed.rs
@@ -10,7 +10,7 @@
 
 //! A simple test for testing many permutations of allowedness of
 //! impl Trait
-#![feature(conservative_impl_trait, nested_impl_trait, universal_impl_trait, dyn_trait)]
+#![feature(conservative_impl_trait, universal_impl_trait, dyn_trait)]
 use std::fmt::Debug;
 
 // Allowed
@@ -60,6 +60,7 @@ fn in_dyn_Fn_return_in_return() -> &'static dyn Fn() -> impl Debug { panic!() }
 // Disallowed
 fn in_impl_Fn_parameter_in_parameters(_: &impl Fn(impl Debug)) { panic!() }
 //~^ ERROR `impl Trait` not allowed outside of function and inherent method return types
+//~^^ ERROR nested `impl Trait` is not allowed
 
 // Disallowed
 fn in_impl_Fn_return_in_parameters(_: &impl Fn() -> impl Debug) { panic!() }
@@ -68,6 +69,7 @@ fn in_impl_Fn_return_in_parameters(_: &impl Fn() -> impl Debug) { panic!() }
 // Disallowed
 fn in_impl_Fn_parameter_in_return() -> &'static impl Fn(impl Debug) { panic!() }
 //~^ ERROR `impl Trait` not allowed outside of function and inherent method return types
+//~^^ ERROR nested `impl Trait` is not allowed
 
 // Disallowed
 fn in_impl_Fn_return_in_return() -> &'static impl Fn() -> impl Debug { panic!() }
diff --git a/src/test/run-pass/impl-trait/lifetimes.rs b/src/test/run-pass/impl-trait/lifetimes.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(conservative_impl_trait, underscore_lifetimes, universal_impl_trait, nested_impl_trait)]
+#![feature(conservative_impl_trait, underscore_lifetimes, universal_impl_trait)]
 #![allow(warnings)]
 
 use std::fmt::Debug;
@@ -63,12 +63,11 @@ fn pass_through_elision_with_fn_ptr(x: &fn(&u32) -> &u32) -> impl Into<&fn(&u32)
 
 fn pass_through_elision_with_fn_path<T: Fn(&u32) -> &u32>(
     x: &T
-) -> impl Into<&impl Fn(&u32) -> &u32> { x }
+) -> &impl Fn(&u32) -> &u32 { x }
 
-fn foo(x: &impl Debug) -> impl Into<&impl Debug> { x }
-fn foo_explicit_lifetime<'a>(x: &'a impl Debug) -> impl Into<&'a impl Debug> { x }
-fn foo_no_outer_impl(x: &impl Debug) -> &impl Debug { x }
-fn foo_explicit_arg<T: Debug>(x: &T) -> impl Into<&impl Debug> { x }
+fn foo(x: &impl Debug) -> &impl Debug { x }
+fn foo_explicit_lifetime<'a>(x: &'a impl Debug) -> &'a impl Debug { x }
+fn foo_explicit_arg<T: Debug>(x: &T) -> &impl Debug { x }
 
 fn mixed_lifetimes<'a>() -> impl for<'b: 'a> Fn(&'b u32) { |_| () }
 fn mixed_as_static() -> impl Fn(&'static u32) { mixed_lifetimes() }
diff --git a/src/test/ui/error-codes/E0657.rs b/src/test/ui/error-codes/E0657.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 #![allow(warnings)]
-#![feature(conservative_impl_trait, nested_impl_trait)]
+#![feature(conservative_impl_trait)]
 
 trait Id<T> {}
 trait Lt<'a> {}
@@ -17,7 +17,7 @@ impl<'a> Lt<'a> for () {}
 impl<T> Id<T> for T {}
 
 fn free_fn_capture_hrtb_in_impl_trait()
-    -> impl for<'a> Id<impl Lt<'a>>
+    -> Box<for<'a> Id<impl Lt<'a>>>
         //~^ ERROR `impl Trait` can only capture lifetimes bound at the fn or impl level [E0657]
 {
     ()
@@ -26,7 +26,7 @@ fn free_fn_capture_hrtb_in_impl_trait()
 struct Foo;
 impl Foo {
     fn impl_fn_capture_hrtb_in_impl_trait()
-        -> impl for<'a> Id<impl Lt<'a>>
+        -> Box<for<'a> Id<impl Lt<'a>>>
             //~^ ERROR `impl Trait` can only capture lifetimes bound at the fn or impl level
     {
         ()
diff --git a/src/test/ui/error-codes/E0657.stderr b/src/test/ui/error-codes/E0657.stderr
@@ -1,14 +1,14 @@
 error[E0657]: `impl Trait` can only capture lifetimes bound at the fn or impl level
-  --> $DIR/E0657.rs:20:32
+  --> $DIR/E0657.rs:20:31
    |
-20 |     -> impl for<'a> Id<impl Lt<'a>>
-   |                                ^^
+20 |     -> Box<for<'a> Id<impl Lt<'a>>>
+   |                               ^^
 
 error[E0657]: `impl Trait` can only capture lifetimes bound at the fn or impl level
-  --> $DIR/E0657.rs:29:36
+  --> $DIR/E0657.rs:29:35
    |
-29 |         -> impl for<'a> Id<impl Lt<'a>>
-   |                                    ^^
+29 |         -> Box<for<'a> Id<impl Lt<'a>>>
+   |                                   ^^
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/impl_trait_projections.rs b/src/test/ui/impl_trait_projections.rs
@@ -0,0 +1,50 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+#![feature(dyn_trait, conservative_impl_trait, universal_impl_trait)]
+
+use std::fmt::Debug;
+use std::option;
+
+fn parametrized_type_is_allowed() -> Option<impl Debug> {
+    Some(5i32)
+}
+
+fn path_parametrized_type_is_allowed() -> option::Option<impl Debug> {
+    Some(5i32)
+}
+
+fn projection_is_disallowed(x: impl Iterator) -> <impl Iterator>::Item {
+//~^ ERROR `impl Trait` is not allowed in path parameters
+//~^^ ERROR ambiguous associated type
+    x.next().unwrap()
+}
+
+fn projection_with_named_trait_is_disallowed(x: impl Iterator)
+    -> <impl Iterator as Iterator>::Item
+//~^ ERROR `impl Trait` is not allowed in path parameters
+{
+    x.next().unwrap()
+}
+
+fn projection_with_named_trait_inside_path_is_disallowed()
+    -> <::std::ops::Range<impl Debug> as Iterator>::Item
+//~^ ERROR `impl Trait` is not allowed in path parameters
+{
+    (1i32..100).next().unwrap()
+}
+
+fn projection_from_impl_trait_inside_dyn_trait_is_disallowed()
+    -> <dyn Iterator<Item = impl Debug> as Iterator>::Item
+//~^ ERROR `impl Trait` is not allowed in path parameters
+{
+    panic!()
+}
+
+fn main() {}
diff --git a/src/test/ui/impl_trait_projections.stderr b/src/test/ui/impl_trait_projections.stderr
diff --git a/src/test/ui/nested_impl_trait.rs b/src/test/ui/nested_impl_trait.rs
diff --git a/src/test/ui/nested_impl_trait.stderr b/src/test/ui/nested_impl_trait.stderr

Original file line number	Diff line number	Diff line change
`@@ -304,4 +304,6 @@ register_diagnostics! {`
`304`	`304`	`E0567, // auto traits can not have generic parameters`
`305`	`305`	`E0568, // auto traits can not have super traits`
`306`	`306`	`E0642, // patterns aren't allowed in methods without bodies`
	`307`	+ E0666, // nested `impl Trait` is illegal
	`308`	+ E0667, // `impl Trait` in projections
`307`	`309`	`}`