syntax: Rewrite parsing of impls

Properly parse impls for the never type `!`
Recover from missing `for` in `impl Trait for Type`
Prohibit inherent default impls and default impls of auto traits
Change wording in more diagnostics to use "auto traits"
Some minor code cleanups in the parser

Author: petrochenkov

src/librustc/hir/lowering.rs

@@ -1502,8 +1502,8 @@ impl<'a> LoweringContext<'a> {
                      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
+        // NOTE: The two last parameters here have to do with impl Trait. If fn_def_id is Some,
+        //       then impl Trait arguments are lowered into generic parameters 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

src/librustc_passes/ast_validation.rs

@@ -215,24 +215,36 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
 
     fn visit_item(&mut self, item: &'a Item) {
         match item.node {
-            ItemKind::Impl(.., Some(..), ref ty, ref impl_items) => {
+            ItemKind::Impl(unsafety, polarity, _, _, Some(..), ref ty, ref impl_items) => {
                 self.invalid_visibility(&item.vis, item.span, None);
                 if ty.node == TyKind::Err {
                     self.err_handler()
                         .struct_span_err(item.span, "`impl Trait for .. {}` is an obsolete syntax")
                         .help("use `auto trait Trait {}` instead").emit();
                 }
+                if unsafety == Unsafety::Unsafe && polarity == ImplPolarity::Negative {
+                    span_err!(self.session, item.span, E0198, "negative impls cannot be unsafe");
+                }
                 for impl_item in impl_items {
                     self.invalid_visibility(&impl_item.vis, impl_item.span, None);
                     if let ImplItemKind::Method(ref sig, _) = impl_item.node {
                         self.check_trait_fn_not_const(sig.constness);
                     }
                 }
             }
-            ItemKind::Impl(.., None, _, _) => {
+            ItemKind::Impl(unsafety, polarity, defaultness, _, None, _, _) => {
                 self.invalid_visibility(&item.vis,
                                         item.span,
                                         Some("place qualifiers on individual impl items instead"));
+                if unsafety == Unsafety::Unsafe {
+                    span_err!(self.session, item.span, E0197, "inherent impls cannot be unsafe");
+                }
+                if polarity == ImplPolarity::Negative {
+                    self.err_handler().span_err(item.span, "inherent impls cannot be negative");
+                }
+                if defaultness == Defaultness::Default {
+                    self.err_handler().span_err(item.span, "inherent impls cannot be default");
+                }
             }
             ItemKind::ForeignMod(..) => {
                 self.invalid_visibility(&item.vis,

src/librustc_passes/diagnostics.rs

@@ -82,6 +82,52 @@ extern {
 ```
 "##,
 
+E0197: r##"
+Inherent implementations (one that do not implement a trait but provide
+methods associated with a type) are always safe because they are not
+implementing an unsafe trait. Removing the `unsafe` keyword from the inherent
+implementation will resolve this error.
+
+```compile_fail,E0197
+struct Foo;
+
+// this will cause this error
+unsafe impl Foo { }
+// converting it to this will fix it
+impl Foo { }
+```
+"##,
+
+E0198: r##"
+A negative implementation is one that excludes a type from implementing a
+particular trait. Not being able to use a trait is always a safe operation,
+so negative implementations are always safe and never need to be marked as
+unsafe.
+
+```compile_fail
+#![feature(optin_builtin_traits)]
+
+struct Foo;
+
+// unsafe is unnecessary
+unsafe impl !Clone for Foo { }
+```
+
+This will compile:
+
+```ignore (ignore auto_trait future compatibility warning)
+#![feature(optin_builtin_traits)]
+
+struct Foo;
+
+auto trait Enterprise {}
+
+impl !Enterprise for Foo { }
+```
+
+Please note that negative impls are only allowed for auto traits.
+"##,
+
 E0265: r##"
 This error indicates that a static or constant references itself.
 All statics and constants need to resolve to a value in an acyclic manner.

src/librustc_typeck/check/wfcheck.rs

@@ -107,16 +107,21 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
             //
             // won't be allowed unless there's an *explicit* implementation of `Send`
             // for `T`
-            hir::ItemImpl(_, hir::ImplPolarity::Positive, _, _,
-                          ref trait_ref, ref self_ty, _) => {
-                self.check_impl(item, self_ty, trait_ref);
-            }
-            hir::ItemImpl(_, hir::ImplPolarity::Negative, _, _, Some(_), ..) => {
-                // FIXME(#27579) what amount of WF checking do we need for neg impls?
-
-                let trait_ref = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id)).unwrap();
-                if !tcx.trait_is_auto(trait_ref.def_id) {
-                    error_192(tcx, item.span);
+            hir::ItemImpl(_, polarity, defaultness, _, ref trait_ref, ref self_ty, _) => {
+                let is_auto = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id))
+                                 .map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id));
+                if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) {
+                    tcx.sess.span_err(item.span, "impls of auto traits cannot be default");
+                }
+                if polarity == hir::ImplPolarity::Positive {
+                    self.check_impl(item, self_ty, trait_ref);
+                } else {
+                    // FIXME(#27579) what amount of WF checking do we need for neg impls?
+                    if trait_ref.is_some() && !is_auto {
+                        span_err!(tcx.sess, item.span, E0192,
+                                  "negative impls are only allowed for \
+                                   auto traits (e.g., `Send` and `Sync`)")
+                    }
                 }
             }
             hir::ItemFn(..) => {
@@ -661,12 +666,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     }
 }
 
-fn error_192(tcx: TyCtxt, span: Span) {
-    span_err!(tcx.sess, span, E0192,
-              "negative impls are only allowed for traits with \
-               default impls (e.g., `Send` and `Sync`)")
-}
-
 fn error_392<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span: Span, param_name: ast::Name)
                        -> DiagnosticBuilder<'tcx> {
     let mut err = struct_span_err!(tcx.sess, span, E0392,

src/librustc_typeck/coherence/inherent_impls.rs

@@ -93,23 +93,11 @@ struct InherentCollect<'a, 'tcx: 'a> {
 
 impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentCollect<'a, 'tcx> {
     fn visit_item(&mut self, item: &hir::Item) {
-        let (unsafety, ty) = match item.node {
-            hir::ItemImpl(unsafety, .., None, ref ty, _) => (unsafety, ty),
+        let ty = match item.node {
+            hir::ItemImpl(.., None, ref ty, _) => ty,
             _ => return
         };
 
-        match unsafety {
-            hir::Unsafety::Normal => {
-                // OK
-            }
-            hir::Unsafety::Unsafe => {
-                span_err!(self.tcx.sess,
-                          item.span,
-                          E0197,
-                          "inherent impls cannot be declared as unsafe");
-            }
-        }
-
         let def_id = self.tcx.hir.local_def_id(item.id);
         let self_ty = self.tcx.type_of(def_id);
         let lang_items = self.tcx.lang_items();

src/librustc_typeck/coherence/orphan.rs

@@ -67,16 +67,15 @@ impl<'cx, 'tcx, 'v> ItemLikeVisitor<'v> for OrphanChecker<'cx, 'tcx> {
                     }
                 }
 
-                // In addition to the above rules, we restrict impls of defaulted traits
+                // In addition to the above rules, we restrict impls of auto traits
                 // so that they can only be implemented on nominal types, such as structs,
                 // enums or foreign types. To see why this restriction exists, consider the
-                // following example (#22978). Imagine that crate A defines a defaulted trait
+                // following example (#22978). Imagine that crate A defines an auto trait
                 // `Foo` and a fn that operates on pairs of types:
                 //
                 // ```
                 // // Crate A
-                // trait Foo { }
-                // impl Foo for .. { }
+                // auto trait Foo { }
                 // fn two_foos<A:Foo,B:Foo>(..) {
                 //     one_foo::<(A,B)>(..)
                 // }

src/librustc_typeck/coherence/unsafety.rs

@@ -37,14 +37,7 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
                 let trait_def = self.tcx.trait_def(trait_ref.def_id);
                 let unsafe_attr = impl_generics.and_then(|g| g.carries_unsafe_attr());
                 match (trait_def.unsafety, unsafe_attr, unsafety, polarity) {
-                    (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
-                        span_err!(self.tcx.sess,
-                                  item.span,
-                                  E0198,
-                                  "negative implementations are not unsafe");
-                    }
-
-                    (Unsafety::Normal, None, Unsafety::Unsafe, _) => {
+                    (Unsafety::Normal, None, Unsafety::Unsafe, hir::ImplPolarity::Positive) => {
                         span_err!(self.tcx.sess,
                                   item.span,
                                   E0199,
@@ -69,6 +62,10 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
                                   g.attr_name());
                     }
 
+                    (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
+                        // Reported in AST validation
+                        self.tcx.sess.delay_span_bug(item.span, "unsafe negative impl");
+                    }
                     (_, _, Unsafety::Normal, hir::ImplPolarity::Negative) |
                     (Unsafety::Unsafe, _, Unsafety::Unsafe, hir::ImplPolarity::Positive) |
                     (Unsafety::Normal, Some(_), Unsafety::Unsafe, hir::ImplPolarity::Positive) |

src/librustc_typeck/diagnostics.rs

@@ -1715,7 +1715,7 @@ type Foo = Trait<Bar=i32>; // ok!
 "##,
 
 E0192: r##"
-Negative impls are only allowed for traits with default impls. For more
+Negative impls are only allowed for auto traits. For more
 information see the [opt-in builtin traits RFC][RFC 19].
 
 [RFC 19]: https://github.com/rust-lang/rfcs/blob/master/text/0019-opt-in-builtin-traits.md
@@ -1821,52 +1821,6 @@ impl Trait for Foo {
 ```
 "##,
 
-E0197: r##"
-Inherent implementations (one that do not implement a trait but provide
-methods associated with a type) are always safe because they are not
-implementing an unsafe trait. Removing the `unsafe` keyword from the inherent
-implementation will resolve this error.
-
-```compile_fail,E0197
-struct Foo;
-
-// this will cause this error
-unsafe impl Foo { }
-// converting it to this will fix it
-impl Foo { }
-```
-"##,
-
-E0198: r##"
-A negative implementation is one that excludes a type from implementing a
-particular trait. Not being able to use a trait is always a safe operation,
-so negative implementations are always safe and never need to be marked as
-unsafe.
-
-```compile_fail
-#![feature(optin_builtin_traits)]
-
-struct Foo;
-
-// unsafe is unnecessary
-unsafe impl !Clone for Foo { }
-```
-
-This will compile:
-
-```
-#![feature(optin_builtin_traits)]
-
-struct Foo;
-
-auto trait Enterprise {}
-
-impl !Enterprise for Foo { }
-```
-
-Please note that negative impls are only allowed for traits with default impls.
-"##,
-
 E0199: r##"
 Safe traits should not have unsafe implementations, therefore marking an
 implementation for a safe trait unsafe will cause a compiler error. Removing