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 (#37653 (comment), #37653 (comment))
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 #38268 with actually parsing generic parameters
Add a test for #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,
-        //       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