Incorporate review feedback

Add requested comments, restructure some small bits of code. Fix extern
declarations allowing impl Trait.

3 files changed, 22 insertions, 11 deletions

diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 723f50faa85..e47b15e3781 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -1130,11 +1130,19 @@ impl<'a> LoweringContext<'a> {
         }).collect()
     }
 
+
     fn lower_fn_decl(&mut self,
                      decl: &FnDecl,
                      fn_def_id: Option<DefId>,
                      impl_trait_return_allow: bool)
                      -> P<hir::FnDecl> {
+        // NOTE: The two last paramters here have to do with impl Trait. If fn_def_id is Some,
+        //       then impl Trait arguments are lowered into generic paramters on the given
+        //       fn_def_id, otherwise impl Trait is disallowed. (for now)
+        //
+        //       Furthermore, if impl_trait_return_allow is true, then impl Trait may be used in
+        //       return positions as well. This guards against trait declarations and their impls
+        //       where impl Trait is disallowed. (again for now)
         P(hir::FnDecl {
             inputs: decl.inputs.iter()
                 .map(|arg| if let Some(def_id) = fn_def_id {
@@ -1143,9 +1151,9 @@ impl<'a> LoweringContext<'a> {
                     self.lower_ty(&arg.ty, ImplTraitContext::Disallowed)
                 }).collect(),
             output: match decl.output {
-                FunctionRetTy::Ty(ref ty) => match (impl_trait_return_allow, fn_def_id) {
-                    (false, _) => hir::Return(self.lower_ty(ty, ImplTraitContext::Disallowed)),
-                    (_, Some(_)) => hir::Return(self.lower_ty(ty, ImplTraitContext::Existential)),
+                FunctionRetTy::Ty(ref ty) => match fn_def_id {
+                    Some(_) if impl_trait_return_allow =>
+                        hir::Return(self.lower_ty(ty, ImplTraitContext::Existential)),
                     _ => hir::Return(self.lower_ty(ty, ImplTraitContext::Disallowed)),
                 },
                 FunctionRetTy::Default(span) => hir::DefaultReturn(span),
@@ -1730,7 +1738,8 @@ impl<'a> LoweringContext<'a> {
                             this.expr_block(body, ThinVec::new())
                         });
                         let impl_trait_return_allow = !this.is_in_trait_impl;
-                        hir::ImplItemKind::Method(this.lower_method_sig(sig, fn_def_id,
+                        hir::ImplItemKind::Method(this.lower_method_sig(sig,
+                                                                        fn_def_id,
                                                                         impl_trait_return_allow),
                                                   body_id)
                     }
@@ -1833,8 +1842,8 @@ impl<'a> LoweringContext<'a> {
                 attrs: this.lower_attrs(&i.attrs),
                 node: match i.node {
                     ForeignItemKind::Fn(ref fdec, ref generics) => {
-                        let fn_def_id = this.resolver.definitions().opt_local_def_id(i.id);
-                        hir::ForeignItemFn(this.lower_fn_decl(fdec, fn_def_id, true),
+                        // Disallow impl Trait in foreign items
+                        hir::ForeignItemFn(this.lower_fn_decl(fdec, None, false),
                                            this.lower_fn_args_to_names(fdec),
                                            this.lower_generics(generics))
                     }
diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs
index 76afd4e2bd1..b5fbbeb1692 100644
--- a/src/librustc_typeck/collect.rs
+++ b/src/librustc_typeck/collect.rs
@@ -1763,9 +1763,13 @@ fn extract_universal_impl_trait_info<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     }
 
     let mut visitor = ImplTraitUniversalVisitor { items: Vec::new() };
-    opt_inputs.map(|inputs| for t in inputs.iter() {
-        visitor.visit_ty(t);
-    });
+
+    if let Some(inputs) = opt_inputs {
+        for t in inputs.iter() {
+            visitor.visit_ty(t);
+        }
+    }
+
     visitor.items.into_iter().map(|ty| if let hir::TyImplTraitUniversal(_, ref bounds) = ty.node {
         ImplTraitUniversalInfo {
             id: ty.id,
diff --git a/src/test/compile-fail/impl-trait/where-allowed.rs b/src/test/compile-fail/impl-trait/where-allowed.rs
index be990b0e15d..a4361446020 100644
--- a/src/test/compile-fail/impl-trait/where-allowed.rs
+++ b/src/test/compile-fail/impl-trait/where-allowed.rs
@@ -144,11 +144,9 @@ impl DummyType {
 extern "C" {
     fn in_foreign_parameters(_: impl Debug);
     //~^ ERROR `impl Trait` not allowed outside of function and inherent method return types
-    // FIXME currently allowed
 
     fn in_foreign_return() -> impl Debug;
     //~^ ERROR `impl Trait` not allowed outside of function and inherent method return types
-    // FIXME currently allowed
 }
 
 // Allowed