Auto merge of #46455 - petrochenkov:pimpl, r=nikomatsakis

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 (https://github.com/rust-lang/rust/issues/37653#issuecomment-348687794, https://github.com/rust-lang/rust/issues/37653#issuecomment-348688785)
Change wording in more diagnostics to use "auto traits"
Fix some spans in diagnostics
Some other minor code cleanups in the parser
Disambiguate generics and qualified paths in impls (parse `impl <Type as Trait>::AssocTy { ... }`)
Replace the future-compatibility hack from https://github.com/rust-lang/rust/pull/38268 with actually parsing generic parameters
Add a test for https://github.com/rust-lang/rust/issues/46438

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,
+        // 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 paramters on the given
+        //       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,13 +215,16 @@ 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 {
@@ -229,10 +232,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     }
                 }
             }
-            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, _, _,
+            hir::ItemImpl(_, polarity, defaultness, _, ref trait_ref, ref self_ty, _) => {
-                          ref trait_ref, ref self_ty, _) => {
+                let is_auto = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id))
-                self.check_impl(item, self_ty, trait_ref);
+                                 .map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id));
-            }
+                if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) {
-            hir::ItemImpl(_, hir::ImplPolarity::Negative, _, _, Some(_), ..) => {
+                    tcx.sess.span_err(item.span, "impls of auto traits cannot be default");
-                // FIXME(#27579) what amount of WF checking do we need for neg impls?
+                }
-
+                if polarity == hir::ImplPolarity::Positive {
-                let trait_ref = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id)).unwrap();
+                    self.check_impl(item, self_ty, trait_ref);
-                if !tcx.trait_is_auto(trait_ref.def_id) {
+                } else {
-                    error_192(tcx, item.span);
+                    // 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 {
+        let ty = match item.node {
-            hir::ItemImpl(unsafety, .., None, ref ty, _) => (unsafety, ty),
+            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 { }
+                // auto trait Foo { }
-                // impl Foo for .. { }
                 // 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) => {
+                    (Unsafety::Normal, None, Unsafety::Unsafe, hir::ImplPolarity::Positive) => {
-                        span_err!(self.tcx.sess,
-                                  item.span,
-                                  E0198,
-                                  "negative implementations are not unsafe");
-                    }
-
-                    (Unsafety::Normal, None, Unsafety::Unsafe, _) => {
                         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

src/libsyntax/parse/parser.rs

@@ -71,7 +71,7 @@ bitflags! {
     }
 }
 
-type ItemInfo = (Ident, ItemKind, Option<Vec<Attribute> >);
+type ItemInfo = (Ident, ItemKind, Option<Vec<Attribute>>);
 
 /// How to parse a path.
 #[derive(Copy, Clone, PartialEq)]
@@ -151,10 +151,9 @@ macro_rules! maybe_whole {
     };
 }
 
-fn maybe_append(mut lhs: Vec<Attribute>, rhs: Option<Vec<Attribute>>)
+fn maybe_append(mut lhs: Vec<Attribute>, mut rhs: Option<Vec<Attribute>>) -> Vec<Attribute> {
-                -> Vec<Attribute> {
+    if let Some(ref mut rhs) = rhs {
-    if let Some(ref attrs) = rhs {
+        lhs.append(rhs);
-        lhs.extend(attrs.iter().cloned())
     }
     lhs
 }
@@ -1347,7 +1346,7 @@ impl<'a> Parser<'a> {
         Function Style
         */
 
-        let unsafety = self.parse_unsafety()?;
+        let unsafety = self.parse_unsafety();
         let abi = if self.eat_keyword(keywords::Extern) {
             self.parse_opt_abi()?.unwrap_or(Abi::C)
         } else {
@@ -1370,11 +1369,12 @@ impl<'a> Parser<'a> {
         })))
     }
 
-    pub fn parse_unsafety(&mut self) -> PResult<'a, Unsafety> {
+    /// Parse unsafety: `unsafe` or nothing.
+    fn parse_unsafety(&mut self) -> Unsafety {
         if self.eat_keyword(keywords::Unsafe) {
-            return Ok(Unsafety::Unsafe);
+            Unsafety::Unsafe
         } else {
-            return Ok(Unsafety::Normal);
+            Unsafety::Normal
         }
     }
 
@@ -4094,28 +4094,6 @@ impl<'a> Parser<'a> {
          self.look_ahead(2, |t| t.is_keyword(keywords::Trait)))
     }
 
-    fn is_defaultness(&self) -> bool {
-        // `pub` is included for better error messages
-        self.token.is_keyword(keywords::Default) &&
-        self.look_ahead(1, |t| t.is_keyword(keywords::Impl) ||
-                        t.is_keyword(keywords::Const) ||
-                        t.is_keyword(keywords::Fn) ||
-                        t.is_keyword(keywords::Unsafe) ||
-                        t.is_keyword(keywords::Extern) ||
-                        t.is_keyword(keywords::Type) ||
-                        t.is_keyword(keywords::Pub))
-    }
-
-    fn eat_defaultness(&mut self) -> bool {
-        let is_defaultness = self.is_defaultness();
-        if is_defaultness {
-            self.bump()
-        } else {
-            self.expected_tokens.push(TokenType::Keyword(keywords::Default));
-        }
-        is_defaultness
-    }
-
     fn eat_macro_def(&mut self, attrs: &[Attribute], vis: &Visibility, lo: Span)
                      -> PResult<'a, Option<P<Item>>> {
         let token_lo = self.span;
@@ -4794,21 +4772,13 @@ impl<'a> Parser<'a> {
         }
         let lo = self.prev_span;
 
-        // This is a temporary future proofing.
-        //
         // We are considering adding generics to the `where` keyword as an alternative higher-rank
         // parameter syntax (as in `where<'a>` or `where<T>`. To avoid that being a breaking
-        // change, for now we refuse to parse `where < (ident | lifetime) (> | , | :)`.
+        // change we parse those generics now, but report an error.
-        if token::Lt == self.token {
+        if self.choose_generics_over_qpath() {
-            let ident_or_lifetime = self.look_ahead(1, |t| t.is_ident() || t.is_lifetime());
+            let generics = self.parse_generics()?;
-            if ident_or_lifetime {
+            self.span_err(generics.span,
-                let gt_comma_or_colon = self.look_ahead(2, |t| {
+                          "generic parameters on `where` clauses are reserved for future use");
-                    *t == token::Gt || *t == token::Comma || *t == token::Colon
-                });
-                if gt_comma_or_colon {
-                    self.span_err(self.span, "syntax `where<T>` is reserved for future use");
-                }
-            }
         }
 
         loop {
@@ -5126,7 +5096,7 @@ impl<'a> Parser<'a> {
     fn parse_item_fn(&mut self,
                      unsafety: Unsafety,
                      constness: Spanned<Constness>,
-                     abi: abi::Abi)
+                     abi: Abi)
                      -> PResult<'a, ItemInfo> {
         let (ident, mut generics) = self.parse_fn_header()?;
         let decl = self.parse_fn_decl(false)?;
@@ -5150,13 +5120,10 @@ impl<'a> Parser<'a> {
     /// - `const unsafe fn`
     /// - `extern fn`
     /// - etc
-    pub fn parse_fn_front_matter(&mut self)
+    pub fn parse_fn_front_matter(&mut self) -> PResult<'a, (Spanned<Constness>, Unsafety, Abi)> {
-                                 -> PResult<'a, (Spanned<ast::Constness>,
-                                                ast::Unsafety,
-                                                abi::Abi)> {
         let is_const_fn = self.eat_keyword(keywords::Const);
         let const_span = self.prev_span;
-        let unsafety = self.parse_unsafety()?;
+        let unsafety = self.parse_unsafety();
         let (constness, unsafety, abi) = if is_const_fn {
             (respan(const_span, Constness::Const), unsafety, Abi::Rust)
         } else {
@@ -5191,7 +5158,7 @@ impl<'a> Parser<'a> {
                         mut attrs: Vec<Attribute>) -> PResult<'a, ImplItem> {
         let lo = self.span;
         let vis = self.parse_visibility(false)?;
-        let defaultness = self.parse_defaultness()?;
+        let defaultness = self.parse_defaultness();
         let (name, node, generics) = if self.eat_keyword(keywords::Type) {
             // This parses the grammar:
             //     ImplItemAssocTy = Ident ["<"...">"] ["where" ...] "=" Ty ";"
@@ -5284,7 +5251,7 @@ impl<'a> Parser<'a> {
 
     /// Parse a method or a macro invocation in a trait impl.
     fn parse_impl_method(&mut self, vis: &Visibility, at_end: &mut bool)
-                         -> PResult<'a, (Ident, Vec<ast::Attribute>, ast::Generics,
+                         -> PResult<'a, (Ident, Vec<Attribute>, ast::Generics,
                              ast::ImplItemKind)> {
         // code copied from parse_macro_use_or_failure... abstraction!
         if self.token.is_path_start() && !self.is_extern_non_path() {
@@ -5373,83 +5340,123 @@ impl<'a> Parser<'a> {
         }
     }
 
-    /// Parses items implementations variants
+    fn choose_generics_over_qpath(&self) -> bool {
-    ///    impl<T> Foo { ... }
+        // There's an ambiguity between generic parameters and qualified paths in impls.
-    ///    impl<T> ToString for &'static T { ... }
+        // If we see `<` it may start both, so we have to inspect some following tokens.
-    fn parse_item_impl(&mut self,
+        // The following combinations can only start generics,
-                       unsafety: ast::Unsafety,
+        // but not qualified paths (with one exception):
-                       defaultness: Defaultness) -> PResult<'a, ItemInfo> {
+        //     `<` `>` - empty generic parameters
-
+        //     `<` `#` - generic parameters with attributes
-        // First, parse type parameters if necessary.
+        //     `<` (LIFETIME|IDENT) `>` - single generic parameter
-        let mut generics = self.parse_generics()?;
+        //     `<` (LIFETIME|IDENT) `,` - first generic parameter in a list
-
+        //     `<` (LIFETIME|IDENT) `:` - generic parameter with bounds
-        // Special case: if the next identifier that follows is '(', don't
+        //     `<` (LIFETIME|IDENT) `=` - generic parameter with a default
-        // allow this to be parsed as a trait.
+        // The only truly ambiguous case is
-        let could_be_trait = self.token != token::OpenDelim(token::Paren);
+        //     `<` IDENT `>` `::` IDENT ...
-
+        // we disambiguate it in favor of generics (`impl<T> ::absolute::Path<T> { ... }`)
-        let neg_span = self.span;
+        // because this is what almost always expected in practice, qualified paths in impls
-        let polarity = if self.eat(&token::Not) {
+        // (`impl <Type>::AssocTy { ... }`) aren't even allowed by type checker at the moment.
-            ast::ImplPolarity::Negative
+        self.token == token::Lt &&
-        } else {
+            (self.look_ahead(1, |t| t == &token::Pound || t == &token::Gt) ||
-            ast::ImplPolarity::Positive
+             self.look_ahead(1, |t| t.is_lifetime() || t.is_ident()) &&
-        };
+                self.look_ahead(2, |t| t == &token::Gt || t == &token::Comma ||
-
+                                       t == &token::Colon || t == &token::Eq))
-        // Parse the trait.
+    }
-        let mut ty = self.parse_ty()?;
-
-        // Parse traits, if necessary.
-        let opt_trait = if could_be_trait && self.eat_keyword(keywords::For) {
-            // New-style trait. Reinterpret the type as a trait.
-            match ty.node {
-                TyKind::Path(None, ref path) => {
-                    Some(TraitRef {
-                        path: (*path).clone(),
-                        ref_id: ty.id,
-                    })
-                }
-                _ => {
-                    self.span_err(ty.span, "not a trait");
-                    None
-                }
-            }
-        } else {
-            if polarity == ast::ImplPolarity::Negative {
-                // This is a negated type implementation
-                // `impl !MyType {}`, which is not allowed.
-                self.span_err(neg_span, "inherent implementation can't be negated");
-            }
-            None
-        };
-
-        if opt_trait.is_some() {
-            ty = if self.eat(&token::DotDot) {
-                P(Ty { node: TyKind::Err, span: self.prev_span, id: ast::DUMMY_NODE_ID })
-            } else {
-                self.parse_ty()?
-            }
-        }
-        generics.where_clause = self.parse_where_clause()?;
 
+    fn parse_impl_body(&mut self) -> PResult<'a, (Vec<ImplItem>, Vec<Attribute>)> {
         self.expect(&token::OpenDelim(token::Brace))?;
         let attrs = self.parse_inner_attributes()?;
 
-        let mut impl_items = vec![];
+        let mut impl_items = Vec::new();
         while !self.eat(&token::CloseDelim(token::Brace)) {
             let mut at_end = false;
             match self.parse_impl_item(&mut at_end) {
-                Ok(item) => impl_items.push(item),
+                Ok(impl_item) => impl_items.push(impl_item),
-                Err(mut e) => {
+                Err(mut err) => {
-                    e.emit();
+                    err.emit();
                     if !at_end {
                         self.recover_stmt_(SemiColonMode::Break, BlockMode::Break);
                     }
                 }
             }
         }
+        Ok((impl_items, attrs))
+    }
 
-        Ok((keywords::Invalid.ident(),
+    /// Parses an implementation item, `impl` keyword is already parsed.
-            ItemKind::Impl(unsafety, polarity, defaultness, generics, opt_trait, ty, impl_items),
+    ///    impl<'a, T> TYPE { /* impl items */ }
-            Some(attrs)))
+    ///    impl<'a, T> TRAIT for TYPE { /* impl items */ }
+    ///    impl<'a, T> !TRAIT for TYPE { /* impl items */ }
+    /// We actually parse slightly more relaxed grammar for better error reporting and recovery.
+    ///     `impl` GENERICS `!`? TYPE `for`? (TYPE | `..`) (`where` PREDICATES)? `{` BODY `}`
+    ///     `impl` GENERICS `!`? TYPE (`where` PREDICATES)? `{` BODY `}`
+    fn parse_item_impl(&mut self, unsafety: Unsafety, defaultness: Defaultness)
+                       -> PResult<'a, ItemInfo> {
+        // First, parse generic parameters if necessary.
+        let mut generics = if self.choose_generics_over_qpath() {
+            self.parse_generics()?
+        } else {
+            ast::Generics::default()
+        };
+
+        // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type.
+        let polarity = if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) {
+            self.bump(); // `!`
+            ast::ImplPolarity::Negative
+        } else {
+            ast::ImplPolarity::Positive
+        };
+
+        // Parse both types and traits as a type, then reinterpret if necessary.
+        let ty_first = self.parse_ty()?;
+
+        // If `for` is missing we try to recover.
+        let has_for = self.eat_keyword(keywords::For);
+        let missing_for_span = self.prev_span.between(self.span);
+
+        let ty_second = if self.token == token::DotDot {
+            // We need to report this error after `cfg` expansion for compatibility reasons
+            self.bump(); // `..`, do not add it to expected tokens
+            Some(P(Ty { node: TyKind::Err, span: self.prev_span, id: ast::DUMMY_NODE_ID }))
+        } else if has_for || self.token.can_begin_type() {
+            Some(self.parse_ty()?)
+        } else {
+            None
+        };
+
+        generics.where_clause = self.parse_where_clause()?;
+
+        let (impl_items, attrs) = self.parse_impl_body()?;
+
+        let item_kind = match ty_second {
+            Some(ty_second) => {
+                // impl Trait for Type
+                if !has_for {
+                    self.span_err(missing_for_span, "missing `for` in a trait impl");
+                }
+
+                let ty_first = ty_first.into_inner();
+                let path = match ty_first.node {
+                    // This notably includes paths passed through `ty` macro fragments (#46438).
+                    TyKind::Path(None, path) => path,
+                    _ => {
+                        self.span_err(ty_first.span, "expected a trait, found type");
+                        ast::Path::from_ident(ty_first.span, keywords::Invalid.ident())
+                    }
+                };
+                let trait_ref = TraitRef { path, ref_id: ty_first.id };
+
+                ItemKind::Impl(unsafety, polarity, defaultness,
+                               generics, Some(trait_ref), ty_second, impl_items)
+            }
+            None => {
+                // impl Type
+                ItemKind::Impl(unsafety, polarity, defaultness,
+                               generics, None, ty_first, impl_items)
+            }
+        };
+
+        Ok((keywords::Invalid.ident(), item_kind, Some(attrs)))
     }
 
     fn parse_late_bound_lifetime_defs(&mut self) -> PResult<'a, Vec<GenericParam>> {
@@ -5722,12 +5729,21 @@ impl<'a> Parser<'a> {
         Ok(Visibility::Public)
     }
 
-    /// Parse defaultness: DEFAULT or nothing
+    /// Parse defaultness: `default` or nothing.
-    fn parse_defaultness(&mut self) -> PResult<'a, Defaultness> {
+    fn parse_defaultness(&mut self) -> Defaultness {
-        if self.eat_defaultness() {
+        // `pub` is included for better error messages
-            Ok(Defaultness::Default)
+        if self.check_keyword(keywords::Default) &&
+           self.look_ahead(1, |t| t.is_keyword(keywords::Impl) ||
+                                  t.is_keyword(keywords::Const) ||
+                                  t.is_keyword(keywords::Fn) ||
+                                  t.is_keyword(keywords::Unsafe) ||
+                                  t.is_keyword(keywords::Extern) ||
+                                  t.is_keyword(keywords::Type) ||
+                                  t.is_keyword(keywords::Pub)) {
+            self.bump(); // `default`
+            Defaultness::Default
         } else {
-            Ok(Defaultness::Final)
+            Defaultness::Final
         }
     }
 
@@ -5797,7 +5813,7 @@ impl<'a> Parser<'a> {
                 let (module, mut attrs) =
                     self.eval_src_mod(path, directory_ownership, id.to_string(), id_span)?;
                 if warn {
-                    let attr = ast::Attribute {
+                    let attr = Attribute {
                         id: attr::mk_attr_id(),
                         style: ast::AttrStyle::Outer,
                         path: ast::Path::from_ident(syntax_pos::DUMMY_SP,
@@ -5837,7 +5853,7 @@ impl<'a> Parser<'a> {
         }
     }
 
-    pub fn submod_path_from_attr(attrs: &[ast::Attribute], dir_path: &Path) -> Option<PathBuf> {
+    pub fn submod_path_from_attr(attrs: &[Attribute], dir_path: &Path) -> Option<PathBuf> {
         attr::first_attr_value_str_by_name(attrs, "path").map(|d| dir_path.join(&d.as_str()))
     }
 
@@ -5906,7 +5922,7 @@ impl<'a> Parser<'a> {
 
     fn submod_path(&mut self,
                    id: ast::Ident,
-                   outer_attrs: &[ast::Attribute],
+                   outer_attrs: &[Attribute],
                    id_sp: Span)
                    -> PResult<'a, ModulePathSuccess> {
         if let Some(path) = Parser::submod_path_from_attr(outer_attrs, &self.directory.path) {
@@ -5999,7 +6015,7 @@ impl<'a> Parser<'a> {
                     directory_ownership: DirectoryOwnership,
                     name: String,
                     id_sp: Span)
-                    -> PResult<'a, (ast::ItemKind, Vec<ast::Attribute> )> {
+                    -> PResult<'a, (ast::ItemKind, Vec<Attribute> )> {
         let mut included_mod_stack = self.sess.included_mod_stack.borrow_mut();
         if let Some(i) = included_mod_stack.iter().position(|p| *p == path) {
             let mut err = String::from("circular modules: ");
@@ -6122,7 +6138,7 @@ impl<'a> Parser<'a> {
     /// extern {}
     fn parse_item_foreign_mod(&mut self,
                               lo: Span,
-                              opt_abi: Option<abi::Abi>,
+                              opt_abi: Option<Abi>,
                               visibility: Visibility,
                               mut attrs: Vec<Attribute>)
                               -> PResult<'a, P<Item>> {
@@ -6225,7 +6241,7 @@ impl<'a> Parser<'a> {
 
     /// Parses a string as an ABI spec on an extern type or module. Consumes
     /// the `extern` keyword, if one is found.
-    fn parse_opt_abi(&mut self) -> PResult<'a, Option<abi::Abi>> {
+    fn parse_opt_abi(&mut self) -> PResult<'a, Option<Abi>> {
         match self.token {
             token::Literal(token::Str_(s), suf) | token::Literal(token::StrRaw(s, _), suf) => {
                 let sp = self.span;
@@ -6330,11 +6346,7 @@ impl<'a> Parser<'a> {
                 || (self.check_keyword(keywords::Unsafe)
                     && self.look_ahead(1, |t| t.is_keyword(keywords::Fn))) {
                 // CONST FUNCTION ITEM
-                let unsafety = if self.eat_keyword(keywords::Unsafe) {
+                let unsafety = self.parse_unsafety();
-                    Unsafety::Unsafe
-                } else {
-                    Unsafety::Normal
-                };
                 self.bump();
                 let (ident, item_, extra_attrs) =
                     self.parse_item_fn(unsafety,
@@ -6370,7 +6382,7 @@ impl<'a> Parser<'a> {
             self.look_ahead(1, |t| t.is_keyword(keywords::Auto)))
         {
             // UNSAFE TRAIT ITEM
-            self.expect_keyword(keywords::Unsafe)?;
+            self.bump(); // `unsafe`
             let is_auto = if self.eat_keyword(keywords::Trait) {
                 IsAuto::No
             } else {
@@ -6379,7 +6391,7 @@ impl<'a> Parser<'a> {
                 IsAuto::Yes
             };
             let (ident, item_, extra_attrs) =
-                self.parse_item_trait(is_auto, ast::Unsafety::Unsafe)?;
+                self.parse_item_trait(is_auto, Unsafety::Unsafe)?;
             let prev_span = self.prev_span;
             let item = self.mk_item(lo.to(prev_span),
                                     ident,
@@ -6388,26 +6400,21 @@ impl<'a> Parser<'a> {
                                     maybe_append(attrs, extra_attrs));
             return Ok(Some(item));
         }
-        if (self.check_keyword(keywords::Unsafe) &&
+        if self.check_keyword(keywords::Impl) ||
-            self.look_ahead(1, |t| t.is_keyword(keywords::Impl))) ||
+           self.check_keyword(keywords::Unsafe) &&
-           (self.check_keyword(keywords::Default) &&
+                self.look_ahead(1, |t| t.is_keyword(keywords::Impl)) ||
-            self.look_ahead(1, |t| t.is_keyword(keywords::Unsafe)) &&
+           self.check_keyword(keywords::Default) &&
-            self.look_ahead(2, |t| t.is_keyword(keywords::Impl)))
+                self.look_ahead(1, |t| t.is_keyword(keywords::Impl)) ||
-        {
+           self.check_keyword(keywords::Default) &&
+                self.look_ahead(1, |t| t.is_keyword(keywords::Unsafe)) {
             // IMPL ITEM
-            let defaultness = self.parse_defaultness()?;
+            let defaultness = self.parse_defaultness();
-            self.expect_keyword(keywords::Unsafe)?;
+            let unsafety = self.parse_unsafety();
             self.expect_keyword(keywords::Impl)?;
-            let (ident,
+            let (ident, item, extra_attrs) = self.parse_item_impl(unsafety, defaultness)?;
-                 item_,
+            let span = lo.to(self.prev_span);
-                 extra_attrs) = self.parse_item_impl(ast::Unsafety::Unsafe, defaultness)?;
+            return Ok(Some(self.mk_item(span, ident, item, visibility,
-            let prev_span = self.prev_span;
+                                        maybe_append(attrs, extra_attrs))));
-            let item = self.mk_item(lo.to(prev_span),
-                                    ident,
-                                    item_,
-                                    visibility,
-                                    maybe_append(attrs, extra_attrs));
-            return Ok(Some(item));
         }
         if self.check_keyword(keywords::Fn) {
             // FUNCTION ITEM
@@ -6428,7 +6435,7 @@ impl<'a> Parser<'a> {
         if self.check_keyword(keywords::Unsafe)
             && self.look_ahead(1, |t| *t != token::OpenDelim(token::Brace)) {
             // UNSAFE FUNCTION ITEM
-            self.bump();
+            self.bump(); // `unsafe`
             let abi = if self.eat_keyword(keywords::Extern) {
                 self.parse_opt_abi()?.unwrap_or(Abi::C)
             } else {
@@ -6495,25 +6502,7 @@ impl<'a> Parser<'a> {
             };
             // TRAIT ITEM
             let (ident, item_, extra_attrs) =
-                self.parse_item_trait(is_auto, ast::Unsafety::Normal)?;
+                self.parse_item_trait(is_auto, Unsafety::Normal)?;
-            let prev_span = self.prev_span;
-            let item = self.mk_item(lo.to(prev_span),
-                                    ident,
-                                    item_,
-                                    visibility,
-                                    maybe_append(attrs, extra_attrs));
-            return Ok(Some(item));
-        }
-        if (self.check_keyword(keywords::Impl)) ||
-           (self.check_keyword(keywords::Default) &&
-            self.look_ahead(1, |t| t.is_keyword(keywords::Impl)))
-        {
-            // IMPL ITEM
-            let defaultness = self.parse_defaultness()?;
-            self.expect_keyword(keywords::Impl)?;
-            let (ident,
-                 item_,
-                 extra_attrs) = self.parse_item_impl(ast::Unsafety::Normal, defaultness)?;
             let prev_span = self.prev_span;
             let item = self.mk_item(lo.to(prev_span),
                                     ident,

src/test/compile-fail/E0198.rs

@@ -12,7 +12,7 @@
 
 struct Foo;
 
-unsafe impl !Clone for Foo { } //~ ERROR negative implementations are not unsafe [E0198]
+unsafe impl !Send for Foo { } //~ ERROR E0198
 
 fn main() {
 }

src/test/compile-fail/coherence-negative-impls-safe.rs

@@ -15,6 +15,6 @@ use std::marker::Send;
 struct TestType;
 
 unsafe impl !Send for TestType {}
-//~^ ERROR negative implementations are not unsafe
+//~^ ERROR negative impls cannot be unsafe
 
 fn main() {}

src/test/compile-fail/issue-46438.rs

@@ -0,0 +1,23 @@
+// Copyright 2017 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.
+
+macro_rules! m {
+    ($my_type: ty) => {
+        impl $my_type for u8 {}
+    }
+}
+
+trait Trait {}
+
+m!(Tr);
+
+m!(&'static u8); //~ ERROR expected a trait, found type
+
+fn main() {}

src/test/compile-fail/phantom-oibit.rs

@@ -9,8 +9,7 @@
 // except according to those terms.
 
 // Ensure that OIBIT checks `T` when it encounters a `PhantomData<T>` field, instead of checking
-// the `PhantomData<T>` type itself (which almost always implements a "default" trait
+// the `PhantomData<T>` type itself (which almost always implements an auto trait)
-// (`impl Trait for ..`))
 
 #![feature(optin_builtin_traits)]
 

src/test/compile-fail/private-in-public-ill-formed.rs

@@ -21,7 +21,7 @@ mod aliases_pub {
         type AssocAlias = m::Pub3;
     }
 
-    impl (<Priv as PrivTr>::AssocAlias) { //~ ERROR no base type found for inherent implementation
+    impl <Priv as PrivTr>::AssocAlias { //~ ERROR no base type found for inherent implementation
         pub fn f(arg: Priv) {} // private type `aliases_pub::Priv` in public interface
     }
 }
@@ -37,7 +37,7 @@ mod aliases_priv {
         type AssocAlias = Priv3;
     }
 
-    impl (<Priv as PrivTr>::AssocAlias) { //~ ERROR no base type found for inherent implementation
+    impl <Priv as PrivTr>::AssocAlias { //~ ERROR no base type found for inherent implementation
         pub fn f(arg: Priv) {} // OK
     }
 }

src/test/compile-fail/specialization/defaultimpl/validation.rs

@@ -0,0 +1,23 @@
+// Copyright 2017 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(optin_builtin_traits)]
+#![feature(specialization)]
+
+struct S;
+struct Z;
+
+default impl S {} //~ ERROR inherent impls cannot be default
+
+default unsafe impl Send for S {} //~ ERROR impls of auto traits cannot be default
+default impl !Send for Z {} //~ ERROR impls of auto traits cannot be default
+
+trait Tr {}
+default impl !Tr for S {} //~ ERROR negative impls are only allowed for auto traits

src/test/compile-fail/syntax-trait-polarity-feature-gate.rs

@@ -14,7 +14,7 @@ struct TestType;
 
 trait TestTrait {}
 
-unsafe impl !Send for TestType {}
+impl !Send for TestType {}
 //~^ ERROR negative trait bounds
 
 fn main() {}

src/test/compile-fail/syntax-trait-polarity.rs

@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// compile-flags: -Z parse-only -Z continue-parse-after-error
-
 #![feature(optin_builtin_traits)]
 
 use std::marker::Send;
@@ -17,19 +15,23 @@ use std::marker::Send;
 struct TestType;
 
 impl !TestType {}
-//~^ ERROR inherent implementation can't be negated
+//~^ ERROR inherent impls cannot be negative
 
 trait TestTrait {}
 
 unsafe impl !Send for TestType {}
+//~^ ERROR negative impls cannot be unsafe
 impl !TestTrait for TestType {}
+//~^ ERROR negative impls are only allowed for auto traits
 
-struct TestType2<T>;
+struct TestType2<T>(T);
 
 impl<T> !TestType2<T> {}
-//~^ ERROR inherent implementation can't be negated
+//~^ ERROR inherent impls cannot be negative
 
 unsafe impl<T> !Send for TestType2<T> {}
+//~^ ERROR negative impls cannot be unsafe
 impl<T> !TestTrait for TestType2<T> {}
+//~^ ERROR negative impls are only allowed for auto traits
 
 fn main() {}

src/test/compile-fail/trait-safety-inherent-impl.rs

@@ -12,7 +12,7 @@
 
 struct SomeStruct;
 
-unsafe impl SomeStruct { //~ ERROR inherent impls cannot be declared as unsafe
+unsafe impl SomeStruct { //~ ERROR inherent impls cannot be unsafe
     fn foo(self) { }
 }
 

src/test/compile-fail/typeck-negative-impls-builtin.rs

@@ -17,6 +17,6 @@ trait TestTrait {
 }
 
 impl !TestTrait for TestType {}
-//~^ ERROR negative impls are only allowed for traits with default impls (e.g., `Send` and `Sync`)
+//~^ ERROR negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
 
 fn main() {}

src/test/parse-fail/impl-qpath.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 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.
+
+// compile-flags: -Z parse-only
+
+impl <*const u8>::AssocTy {} // OK
+impl <Type as Trait>::AssocTy {} // OK
+impl <'a + Trait>::AssocTy {} // OK
+impl <<Type>::AssocTy>::AssocTy {} // OK
+
+FAIL //~ ERROR

src/test/parse-fail/trait-bounds-not-on-impl.rs

@@ -15,9 +15,7 @@ trait Foo {
 
 struct Bar;
 
-impl Foo + Owned for Bar {
+impl Foo + Owned for Bar { //~ ERROR expected a trait, found type
-//~^ ERROR not a trait
-//~^^ ERROR expected one of `where` or `{`, found `Bar`
 }
 
 fn main() { }

src/test/parse-fail/where_with_bound.rs

@@ -11,6 +11,6 @@
 // compile-flags: -Z parse-only
 
 fn foo<T>() where <T>::Item: ToString, T: Iterator { }
-               //~^ syntax `where<T>` is reserved for future use
+//~^ ERROR generic parameters on `where` clauses are reserved for future use
 
 fn main() {}

src/test/run-pass/issue-25693.rs

@@ -8,16 +8,16 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-pub trait Paramters { type SelfRef; }
+pub trait Parameters { type SelfRef; }
 
 struct RP<'a> { _marker: std::marker::PhantomData<&'a ()> }
 struct BP;
 
-impl<'a> Paramters for RP<'a> { type SelfRef = &'a X<RP<'a>>; }
+impl<'a> Parameters for RP<'a> { type SelfRef = &'a X<RP<'a>>; }
-impl Paramters for BP { type SelfRef = Box<X<BP>>; }
+impl Parameters for BP { type SelfRef = Box<X<BP>>; }
 
 pub struct Y;
-pub enum X<P: Paramters> {
+pub enum X<P: Parameters> {
     Nothing,
     SameAgain(P::SelfRef, Y)
 }

src/test/ui/span/impl-parsing.rs

@@ -0,0 +1,21 @@
+// Copyright 2017 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.
+
+// compile-flags: -Z parse-only -Z continue-parse-after-error
+
+impl ! {} // OK
+impl ! where u8: Copy {} // OK
+
+impl Trait Type {} //~ ERROR missing `for` in a trait impl
+impl Trait .. {} //~ ERROR missing `for` in a trait impl
+impl ?Sized for Type {} //~ ERROR expected a trait, found type
+impl ?Sized for .. {} //~ ERROR expected a trait, found type
+
+default unsafe FAIL //~ ERROR expected `impl`, found `FAIL`

src/test/ui/span/impl-parsing.stderr

@@ -0,0 +1,32 @@
+error: missing `for` in a trait impl
+  --> $DIR/impl-parsing.rs:16:11
+   |
+16 | impl Trait Type {} //~ ERROR missing `for` in a trait impl
+   |           ^
+
+error: missing `for` in a trait impl
+  --> $DIR/impl-parsing.rs:17:11
+   |
+17 | impl Trait .. {} //~ ERROR missing `for` in a trait impl
+   |           ^
+
+error: expected a trait, found type
+  --> $DIR/impl-parsing.rs:18:6
+   |
+18 | impl ?Sized for Type {} //~ ERROR expected a trait, found type
+   |      ^^^^^^
+
+error: expected a trait, found type
+  --> $DIR/impl-parsing.rs:19:6
+   |
+19 | impl ?Sized for .. {} //~ ERROR expected a trait, found type
+   |      ^^^^^^
+
+error: expected `impl`, found `FAIL`
+  --> $DIR/impl-parsing.rs:21:16
+   |
+21 | default unsafe FAIL //~ ERROR expected `impl`, found `FAIL`
+   |                ^^^^ expected `impl` here
+
+error: aborting due to 5 previous errors
+

src/test/ui/typeck-default-trait-impl-outside-crate.stderr

@@ -1,8 +0,0 @@
-error[E0318]: cannot create default implementations for traits outside the crate they're defined in; define a new trait instead
-  --> $DIR/typeck-default-trait-impl-outside-crate.rs:14:6
-   |
-14 | impl Copy for .. {} //~ ERROR E0318
-   |      ^^^^ `Copy` trait not defined in this crate
-
-error: aborting due to previous error
-