Auto merge of #33794 - petrochenkov:sanity, r=nrc

Add AST validation pass and move some checks to it

The purpose of this pass is to catch constructions that fit into AST data structures, but not permitted by the language. As an example, `impl`s don't have visibilities, but for convenience and uniformity with other items they are represented with a structure `Item` which has `Visibility` field.

This pass is intended to run after expansion of macros and syntax extensions (and before lowering to HIR), so it can catch erroneous constructions that were generated by them. This pass allows to remove ad hoc semantic checks from the parser, which can be overruled by syntax extensions and occasionally macros.

The checks can be put here if they are simple, local, don't require results of any complex analysis like name resolution or type checking and maybe don't logically fall into other passes. I expect most of errors generated by this pass to be non-fatal and allowing the compilation to proceed.

I intend to move some more checks to this pass later and maybe extend it with new checks, like, for example, identifier validity. Given that syntax extensions are going to be stabilized in the measurable future, it's important that they would not be able to subvert usual language rules.

In this patch I've added two new checks - a check for labels named `'static` and a check for lifetimes and labels named `'_`. The first one gives a hard error, the second one - a future compatibility warning.
Fixes https://github.com/rust-lang/rust/issues/33059 ([breaking-change])
cc https://github.com/rust-lang/rfcs/pull/1177

r? @nrc

21 files changed, 312 insertions, 214 deletions

diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
index d7971cd2cf0..41086b5d1c9 100644
--- a/src/librustc/lint/builtin.rs
+++ b/src/librustc/lint/builtin.rs
@@ -204,6 +204,12 @@ declare_lint! {
     "object-unsafe non-principal fragments in object types were erroneously allowed"
 }
 
+declare_lint! {
+    pub LIFETIME_UNDERSCORE,
+    Warn,
+    "lifetimes or labels named `'_` were erroneously allowed"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// which are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -242,7 +248,8 @@ impl LintPass for HardwiredLints {
             SUPER_OR_SELF_IN_GLOBAL_PATH,
             UNSIZED_IN_TUPLE,
             OBJECT_UNSAFE_FRAGMENT,
-            HR_LIFETIME_IN_ASSOC_TYPE
+            HR_LIFETIME_IN_ASSOC_TYPE,
+            LIFETIME_UNDERSCORE
         )
     }
 }
diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
index cbbbd84ec50..911626bd2c2 100644
--- a/src/librustc_driver/driver.rs
+++ b/src/librustc_driver/driver.rs
@@ -38,7 +38,7 @@ use rustc_privacy;
 use rustc_plugin::registry::Registry;
 use rustc_plugin as plugin;
 use rustc::hir::lowering::lower_crate;
-use rustc_passes::{no_asm, loops, consts, rvalues, static_recursion};
+use rustc_passes::{ast_validation, no_asm, loops, consts, rvalues, static_recursion};
 use rustc_const_eval::check_match;
 use super::Compilation;
 
@@ -165,6 +165,10 @@ pub fn compile_input(sess: &Session,
              "early lint checks",
              || lint::check_ast_crate(sess, &expanded_crate));
 
+        time(sess.time_passes(),
+             "AST validation",
+             || ast_validation::check_crate(sess, &expanded_crate));
+
         let (analysis, resolutions, mut hir_forest) = {
             lower_and_resolve(sess, &id, &mut defs, &expanded_crate,
                               &sess.dep_graph, control.make_glob_map)
diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs
index 9fca6d3d201..ed12d0d9f3c 100644
--- a/src/librustc_lint/lib.rs
+++ b/src/librustc_lint/lib.rs
@@ -202,6 +202,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             id: LintId::of(HR_LIFETIME_IN_ASSOC_TYPE),
             reference: "issue #33685 <https://github.com/rust-lang/rust/issues/33685>",
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(LIFETIME_UNDERSCORE),
+            reference: "RFC 1177 <https://github.com/rust-lang/rfcs/pull/1177>",
+        },
         ]);
 
     // We have one lint pass defined specially
diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
new file mode 100644
index 00000000000..919c717f888
--- /dev/null
+++ b/src/librustc_passes/ast_validation.rs
@@ -0,0 +1,171 @@
+// Copyright 2016 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.
+
+// Validate AST before lowering it to HIR
+//
+// This pass is supposed to catch things that fit into AST data structures,
+// but not permitted by the language. It runs after expansion when AST is frozen,
+// so it can check for erroneous constructions produced by syntax extensions.
+// This pass is supposed to perform only simple checks not requiring name resolution
+// or type checking or some other kind of complex analysis.
+
+use rustc::lint;
+use rustc::session::Session;
+use syntax::ast::*;
+use syntax::codemap::Span;
+use syntax::errors;
+use syntax::parse::token::{self, keywords};
+use syntax::visit::{self, Visitor};
+
+struct AstValidator<'a> {
+    session: &'a Session,
+}
+
+impl<'a> AstValidator<'a> {
+    fn err_handler(&self) -> &errors::Handler {
+        &self.session.parse_sess.span_diagnostic
+    }
+
+    fn check_label(&self, label: Ident, span: Span, id: NodeId) {
+        if label.name == keywords::StaticLifetime.name() {
+            self.err_handler().span_err(span, &format!("invalid label name `{}`", label.name));
+        }
+        if label.name.as_str() == "'_" {
+            self.session.add_lint(
+                lint::builtin::LIFETIME_UNDERSCORE, id, span,
+                format!("invalid label name `{}`", label.name)
+            );
+        }
+    }
+
+    fn invalid_visibility(&self, vis: &Visibility, span: Span, note: Option<&str>) {
+        if vis != &Visibility::Inherited {
+            let mut err = struct_span_err!(self.session, span, E0449,
+                                           "unnecessary visibility qualifier");
+            if let Some(note) = note {
+                err.span_note(span, note);
+            }
+            err.emit();
+        }
+    }
+}
+
+impl<'a, 'v> Visitor<'v> for AstValidator<'a> {
+    fn visit_lifetime(&mut self, lt: &Lifetime) {
+        if lt.name.as_str() == "'_" {
+            self.session.add_lint(
+                lint::builtin::LIFETIME_UNDERSCORE, lt.id, lt.span,
+                format!("invalid lifetime name `{}`", lt.name)
+            );
+        }
+
+        visit::walk_lifetime(self, lt)
+    }
+
+    fn visit_expr(&mut self, expr: &Expr) {
+        match expr.node {
+            ExprKind::While(_, _, Some(ident)) | ExprKind::Loop(_, Some(ident)) |
+            ExprKind::WhileLet(_, _, _, Some(ident)) | ExprKind::ForLoop(_, _, _, Some(ident)) |
+            ExprKind::Break(Some(ident)) | ExprKind::Again(Some(ident)) => {
+                self.check_label(ident.node, ident.span, expr.id);
+            }
+            _ => {}
+        }
+
+        visit::walk_expr(self, expr)
+    }
+
+    fn visit_path(&mut self, path: &Path, id: NodeId) {
+        if path.global && path.segments.len() > 0 {
+            let ident = path.segments[0].identifier;
+            if token::Ident(ident).is_path_segment_keyword() {
+                self.session.add_lint(
+                    lint::builtin::SUPER_OR_SELF_IN_GLOBAL_PATH, id, path.span,
+                    format!("global paths cannot start with `{}`", ident)
+                );
+            }
+        }
+
+        visit::walk_path(self, path)
+    }
+
+    fn visit_item(&mut self, item: &Item) {
+        match item.node {
+            ItemKind::Use(ref view_path) => {
+                let path = view_path.node.path();
+                if !path.segments.iter().all(|segment| segment.parameters.is_empty()) {
+                    self.err_handler().span_err(path.span, "type or lifetime parameters \
+                                                            in import path");
+                }
+            }
+            ItemKind::Impl(_, _, _, Some(..), _, ref impl_items) => {
+                self.invalid_visibility(&item.vis, item.span, None);
+                for impl_item in impl_items {
+                    self.invalid_visibility(&impl_item.vis, impl_item.span, None);
+                }
+            }
+            ItemKind::Impl(_, _, _, None, _, _) => {
+                self.invalid_visibility(&item.vis, item.span, Some("place qualifiers on individual \
+                                                                    impl items instead"));
+            }
+            ItemKind::DefaultImpl(..) => {
+                self.invalid_visibility(&item.vis, item.span, None);
+            }
+            ItemKind::ForeignMod(..) => {
+                self.invalid_visibility(&item.vis, item.span, Some("place qualifiers on individual \
+                                                                    foreign items instead"));
+            }
+            ItemKind::Enum(ref def, _) => {
+                for variant in &def.variants {
+                    for field in variant.node.data.fields() {
+                        self.invalid_visibility(&field.vis, field.span, None);
+                    }
+                }
+            }
+            _ => {}
+        }
+
+        visit::walk_item(self, item)
+    }
+
+    fn visit_variant_data(&mut self, vdata: &VariantData, _: Ident,
+                          _: &Generics, _: NodeId, span: Span) {
+        if vdata.fields().is_empty() {
+            if vdata.is_tuple() {
+                self.err_handler().struct_span_err(span, "empty tuple structs and enum variants \
+                                                          are not allowed, use unit structs and \
+                                                          enum variants instead")
+                                         .span_help(span, "remove trailing `()` to make a unit \
+                                                           struct or unit enum variant")
+                                         .emit();
+            }
+        }
+
+        visit::walk_struct_def(self, vdata)
+    }
+
+    fn visit_vis(&mut self, vis: &Visibility) {
+        match *vis {
+            Visibility::Restricted{ref path, ..} => {
+                if !path.segments.iter().all(|segment| segment.parameters.is_empty()) {
+                    self.err_handler().span_err(path.span, "type or lifetime parameters \
+                                                            in visibility path");
+                }
+            }
+            _ => {}
+        }
+
+        visit::walk_vis(self, vis)
+    }
+}
+
+pub fn check_crate(session: &Session, krate: &Crate) {
+    visit::walk_crate(&mut AstValidator { session: session }, krate)
+}
diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs
index cba8bd73c01..918e17d21ea 100644
--- a/src/librustc_passes/diagnostics.rs
+++ b/src/librustc_passes/diagnostics.rs
@@ -143,6 +143,46 @@ fn some_func() {
 ```
 "##,
 
+E0449: r##"
+A visibility qualifier was used when it was unnecessary. Erroneous code
+examples:
+
+```compile_fail
+struct Bar;
+
+trait Foo {
+    fn foo();
+}
+
+pub impl Bar {} // error: unnecessary visibility qualifier
+
+pub impl Foo for Bar { // error: unnecessary visibility qualifier
+    pub fn foo() {} // error: unnecessary visibility qualifier
+}
+```
+
+To fix this error, please remove the visibility qualifier when it is not
+required. Example:
+
+```ignore
+struct Bar;
+
+trait Foo {
+    fn foo();
+}
+
+// Directly implemented methods share the visibility of the type itself,
+// so `pub` is unnecessary here
+impl Bar {}
+
+// Trait methods share the visibility of the trait, so `pub` is
+// unnecessary in either case
+pub impl Foo for Bar {
+    pub fn foo() {}
+}
+```
+"##,
+
 }
 
 register_diagnostics! {
diff --git a/src/librustc_passes/lib.rs b/src/librustc_passes/lib.rs
index 67a9c2fd17e..1576ca6bdea 100644
--- a/src/librustc_passes/lib.rs
+++ b/src/librustc_passes/lib.rs
@@ -37,6 +37,7 @@ extern crate rustc_const_math;
 
 pub mod diagnostics;
 
+pub mod ast_validation;
 pub mod consts;
 pub mod loops;
 pub mod no_asm;
diff --git a/src/librustc_privacy/Cargo.toml b/src/librustc_privacy/Cargo.toml
index 0553e54e3aa..ac33c23f023 100644
--- a/src/librustc_privacy/Cargo.toml
+++ b/src/librustc_privacy/Cargo.toml
@@ -9,6 +9,5 @@ path = "lib.rs"
 crate-type = ["dylib"]
 
 [dependencies]
-log = { path = "../liblog" }
 rustc = { path = "../librustc" }
 syntax = { path = "../libsyntax" }
diff --git a/src/librustc_privacy/diagnostics.rs b/src/librustc_privacy/diagnostics.rs
index d33bc52c5a7..36ece929413 100644
--- a/src/librustc_privacy/diagnostics.rs
+++ b/src/librustc_privacy/diagnostics.rs
@@ -115,46 +115,6 @@ pub enum Foo {
 ```
 "##,
 
-E0449: r##"
-A visibility qualifier was used when it was unnecessary. Erroneous code
-examples:
-
-```compile_fail
-struct Bar;
-
-trait Foo {
-    fn foo();
-}
-
-pub impl Bar {} // error: unnecessary visibility qualifier
-
-pub impl Foo for Bar { // error: unnecessary visibility qualifier
-    pub fn foo() {} // error: unnecessary visibility qualifier
-}
-```
-
-To fix this error, please remove the visibility qualifier when it is not
-required. Example:
-
-```ignore
-struct Bar;
-
-trait Foo {
-    fn foo();
-}
-
-// Directly implemented methods share the visibility of the type itself,
-// so `pub` is unnecessary here
-impl Bar {}
-
-// Trait methods share the visibility of the trait, so `pub` is
-// unnecessary in either case
-pub impl Foo for Bar {
-    pub fn foo() {}
-}
-```
-"##,
-
 E0450: r##"
 A tuple constructor was invoked while some of its fields are private. Erroneous
 code example:
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
index c90d152e3c3..7e76842a9f4 100644
--- a/src/librustc_privacy/lib.rs
+++ b/src/librustc_privacy/lib.rs
@@ -21,37 +21,26 @@
 #![feature(rustc_private)]
 #![feature(staged_api)]
 
-#[macro_use] extern crate log;
+extern crate rustc;
 #[macro_use] extern crate syntax;
 
-#[macro_use] extern crate rustc;
-
-use std::cmp;
-use std::mem::replace;
-
+use rustc::dep_graph::DepNode;
 use rustc::hir::{self, PatKind};
+use rustc::hir::def::{self, Def};
+use rustc::hir::def_id::DefId;
 use rustc::hir::intravisit::{self, Visitor};
 use rustc::hir::pat_util::EnumerateAndAdjustIterator;
-use rustc::dep_graph::DepNode;
 use rustc::lint;
-use rustc::hir::def::{self, Def};
-use rustc::hir::def_id::DefId;
 use rustc::middle::privacy::{AccessLevel, AccessLevels};
 use rustc::ty::{self, TyCtxt};
 use rustc::util::nodemap::NodeSet;
-use rustc::hir::map as ast_map;
-
 use syntax::ast;
 use syntax::codemap::Span;
 
-pub mod diagnostics;
-
-type Context<'a, 'tcx> = (&'a ty::MethodMap<'tcx>, &'a def::ExportMap);
+use std::cmp;
+use std::mem::replace;
 
-/// Result of a checking operation - None => no errors were found. Some => an
-/// error and contains the span and message for reporting that error and
-/// optionally the same for a note about the error.
-type CheckResult = Option<(Span, String, Option<(Span, String)>)>;
+pub mod diagnostics;
 
 ////////////////////////////////////////////////////////////////////////////////
 /// The embargo visitor, used to determine the exports of the ast
@@ -433,7 +422,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
             hir::ExprMethodCall(..) => {
                 let method_call = ty::MethodCall::expr(expr.id);
                 let method = self.tcx.tables.borrow().method_map[&method_call];
-                debug!("(privacy checking) checking impl method");
                 self.check_method(expr.span, method.def_id);
             }
             hir::ExprStruct(..) => {
@@ -518,74 +506,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
     }
 }
 
-////////////////////////////////////////////////////////////////////////////////
-/// The privacy sanity check visitor, ensures unnecessary visibility isn't here
-////////////////////////////////////////////////////////////////////////////////
-
-struct SanePrivacyVisitor<'a, 'tcx: 'a> {
-    tcx: TyCtxt<'a, 'tcx, 'tcx>,
-}
-
-impl<'a, 'tcx, 'v> Visitor<'v> for SanePrivacyVisitor<'a, 'tcx> {
-    fn visit_item(&mut self, item: &hir::Item) {
-        self.check_sane_privacy(item);
-        intravisit::walk_item(self, item);
-    }
-}
-
-impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> {
-    /// Validate that items that shouldn't have visibility qualifiers don't have them.
-    /// Such qualifiers can be set by syntax extensions even if the parser doesn't allow them,
-    /// so we check things like variant fields too.
-    fn check_sane_privacy(&self, item: &hir::Item) {
-        let check_inherited = |sp, vis: &hir::Visibility, note: &str| {
-            if *vis != hir::Inherited {
-                let mut err = struct_span_err!(self.tcx.sess, sp, E0449,
-                                               "unnecessary visibility qualifier");
-                if !note.is_empty() {
-                    err.span_note(sp, note);
-                }
-                err.emit();
-            }
-        };
-
-        match item.node {
-            hir::ItemImpl(_, _, _, Some(..), _, ref impl_items) => {
-                check_inherited(item.span, &item.vis,
-                                "visibility qualifiers have no effect on trait impls");
-                for impl_item in impl_items {
-                    check_inherited(impl_item.span, &impl_item.vis,
-                                    "visibility qualifiers have no effect on trait impl items");
-                }
-            }
-            hir::ItemImpl(_, _, _, None, _, _) => {
-                check_inherited(item.span, &item.vis,
-                                "place qualifiers on individual methods instead");
-            }
-            hir::ItemDefaultImpl(..) => {
-                check_inherited(item.span, &item.vis,
-                                "visibility qualifiers have no effect on trait impls");
-            }
-            hir::ItemForeignMod(..) => {
-                check_inherited(item.span, &item.vis,
-                                "place qualifiers on individual functions instead");
-            }
-            hir::ItemEnum(ref def, _) => {
-                for variant in &def.variants {
-                    for field in variant.node.data.fields() {
-                        check_inherited(field.span, &field.vis,
-                                        "visibility qualifiers have no effect on variant fields");
-                    }
-                }
-            }
-            hir::ItemStruct(..) | hir::ItemTrait(..) |
-            hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) |
-            hir::ItemMod(..) | hir::ItemExternCrate(..) |
-            hir::ItemUse(..) | hir::ItemTy(..) => {}
-        }
-    }
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 /// Obsolete visitors for checking for private items in public interfaces.
 /// These visitors are supposed to be kept in frozen state and produce an
@@ -626,7 +546,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
             // .. and it corresponds to a private type in the AST (this returns
             // None for type parameters)
             match self.tcx.map.find(node_id) {
-                Some(ast_map::NodeItem(ref item)) => item.vis != hir::Public,
+                Some(hir::map::NodeItem(ref item)) => item.vis != hir::Public,
                 Some(_) | None => false,
             }
         } else {
@@ -860,7 +780,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx>
         // any `visit_ty`'s will be called on things that are in
         // public signatures, i.e. things that we're interested in for
         // this visitor.
-        debug!("VisiblePrivateTypesVisitor entering item {:?}", item);
         intravisit::walk_item(self, item);
     }
 
@@ -892,7 +811,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx>
     }
 
     fn visit_ty(&mut self, t: &hir::Ty) {
-        debug!("VisiblePrivateTypesVisitor checking ty {:?}", t);
         if let hir::TyPath(..) = t.node {
             if self.path_is_private_type(t.id) {
                 self.old_error_set.insert(t.id);
@@ -1177,10 +1095,6 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     let krate = tcx.map.krate();
 
-    // Sanity check to make sure that all privacy usage is reasonable.
-    let mut visitor = SanePrivacyVisitor { tcx: tcx };
-    krate.visit_all_items(&mut visitor);
-
     // Use the parent map to check the privacy of everything
     let mut visitor = PrivacyVisitor {
         curitem: ast::DUMMY_NODE_ID,
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index e0243bf4fa6..c7b113689fd 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -22,21 +22,20 @@ use Resolver;
 use {resolve_error, resolve_struct_error, ResolutionError};
 
 use rustc::middle::cstore::{ChildItem, DlDef};
-use rustc::lint;
 use rustc::hir::def::*;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::ty::{self, VariantKind};
 
-use syntax::ast::{Name, NodeId};
+use syntax::ast::Name;
 use syntax::attr::AttrMetaMethods;
-use syntax::parse::token::{self, keywords};
+use syntax::parse::token;
 use syntax::codemap::{Span, DUMMY_SP};
 
 use syntax::ast::{Block, Crate, DeclKind};
 use syntax::ast::{ForeignItem, ForeignItemKind, Item, ItemKind};
 use syntax::ast::{Mutability, PathListItemKind};
 use syntax::ast::{Stmt, StmtKind, TraitItemKind};
-use syntax::ast::{Variant, ViewPath, ViewPathGlob, ViewPathList, ViewPathSimple};
+use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
 use syntax::visit::{self, Visitor};
 
 trait ToNameBinding<'a> {
@@ -95,36 +94,6 @@ impl<'b> Resolver<'b> {
         block.stmts.iter().any(is_item)
     }
 
-    fn sanity_check_import(&self, view_path: &ViewPath, id: NodeId) {
-        let path = match view_path.node {
-            ViewPathSimple(_, ref path) |
-            ViewPathGlob (ref path) |
-            ViewPathList(ref path, _) => path
-        };
-
-        // Check for type parameters
-        let found_param = path.segments.iter().any(|segment| {
-            !segment.parameters.types().is_empty() ||
-            !segment.parameters.lifetimes().is_empty() ||
-            !segment.parameters.bindings().is_empty()
-        });
-        if found_param {
-            self.session.span_err(path.span, "type or lifetime parameters in import path");
-        }
-
-        // Checking for special identifiers in path
-        // prevent `self` or `super` at beginning of global path
-        if path.global && path.segments.len() > 0 {
-            let first = path.segments[0].identifier.name;
-            if first == keywords::Super.name() || first == keywords::SelfValue.name() {
-                self.session.add_lint(
-                    lint::builtin::SUPER_OR_SELF_IN_GLOBAL_PATH, id, path.span,
-                    format!("expected identifier, found keyword `{}`", first)
-                );
-            }
-        }
-    }
-
     /// Constructs the reduced graph for one item.
     fn build_reduced_graph_for_item(&mut self, item: &Item, parent_ref: &mut Module<'b>) {
         let parent = *parent_ref;
@@ -158,8 +127,6 @@ impl<'b> Resolver<'b> {
                     }
                 };
 
-                self.sanity_check_import(view_path, item.id);
-
                 // Build up the import directives.
                 let is_prelude = item.attrs.iter().any(|attr| attr.name() == "prelude_import");
 
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 741feb4ba3d..40c98206c16 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -1903,6 +1903,16 @@ pub enum ViewPath_ {
     ViewPathList(Path, Vec<PathListItem>)
 }
 
+impl ViewPath_ {
+    pub fn path(&self) -> &Path {
+        match *self {
+            ViewPathSimple(_, ref path) |
+            ViewPathGlob (ref path) |
+            ViewPathList(ref path, _) => path
+        }
+    }
+}
+
 /// Meta-data associated with an item
 pub type Attribute = Spanned<Attribute_>;
 
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
index 08e593ca747..c11347f5762 100644
--- a/src/libsyntax/feature_gate.rs
+++ b/src/libsyntax/feature_gate.rs
@@ -956,22 +956,6 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
         visit::walk_item(self, i);
     }
 
-    fn visit_variant_data(&mut self, s: &'v ast::VariantData, _: ast::Ident,
-                          _: &'v ast::Generics, _: ast::NodeId, span: Span) {
-        if s.fields().is_empty() {
-            if s.is_tuple() {
-                self.context.span_handler.struct_span_err(span, "empty tuple structs and enum \
-                                                                 variants are not allowed, use \
-                                                                 unit structs and enum variants \
-                                                                 instead")
-                                         .span_help(span, "remove trailing `()` to make a unit \
-                                                           struct or unit enum variant")
-                                         .emit();
-            }
-        }
-        visit::walk_struct_def(self, s)
-    }
-
     fn visit_foreign_item(&mut self, i: &ast::ForeignItem) {
         let links_to_llvm = match attr::first_attr_value_str_by_name(&i.attrs,
                                                                      "link_name") {
@@ -1142,22 +1126,12 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
     fn visit_vis(&mut self, vis: &'v ast::Visibility) {
         let span = match *vis {
             ast::Visibility::Crate(span) => span,
-            ast::Visibility::Restricted { ref path, .. } => {
-                // Check for type parameters
-                let found_param = path.segments.iter().any(|segment| {
-                    !segment.parameters.types().is_empty() ||
-                    !segment.parameters.lifetimes().is_empty() ||
-                    !segment.parameters.bindings().is_empty()
-                });
-                if found_param {
-                    self.context.span_handler.span_err(path.span, "type or lifetime parameters \
-                                                                   in visibility path");
-                }
-                path.span
-            }
+            ast::Visibility::Restricted { ref path, .. } => path.span,
             _ => return,
         };
         gate_feature_post!(&self, pub_restricted, span, "`pub(restricted)` syntax is experimental");
+
+        visit::walk_vis(self, vis)
     }
 }
 
diff --git a/src/rustc/Cargo.lock b/src/rustc/Cargo.lock
index 7a63742fba3..931f90a2394 100644
--- a/src/rustc/Cargo.lock
+++ b/src/rustc/Cargo.lock
@@ -246,7 +246,6 @@ dependencies = [
 name = "rustc_privacy"
 version = "0.0.0"
 dependencies = [
- "log 0.0.0",
  "rustc 0.0.0",
  "syntax 0.0.0",
 ]
diff --git a/src/test/compile-fail/issue-12560-1.rs b/src/test/compile-fail/issue-12560-1.rs
index e005de01649..80f551ebd1f 100644
--- a/src/test/compile-fail/issue-12560-1.rs
+++ b/src/test/compile-fail/issue-12560-1.rs
@@ -21,5 +21,5 @@ enum Foo {
 }
 
 fn main() {
-    println!("{}", match Bar { Bar => 1, Baz => 2, Bazar => 3 })
+    println!("{}", match Bar { Bar => 1, Baz => 2, Bazar => 3 }) //~ ERROR unresolved name `Bar`
 }
diff --git a/src/test/compile-fail/issue-29161.rs b/src/test/compile-fail/issue-29161.rs
index f7453c45be6..bc09f61a754 100644
--- a/src/test/compile-fail/issue-29161.rs
+++ b/src/test/compile-fail/issue-29161.rs
@@ -12,7 +12,7 @@ mod a {
     struct A;
 
     impl Default for A {
-        pub fn default() -> A {
+        pub fn default() -> A { //~ ERROR unnecessary visibility qualifier
             A;
         }
     }
diff --git a/src/test/compile-fail/label-static.rs b/src/test/compile-fail/label-static.rs
new file mode 100644
index 00000000000..a0fb25ea06e
--- /dev/null
+++ b/src/test/compile-fail/label-static.rs
@@ -0,0 +1,15 @@
+// Copyright 2016 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.
+
+fn main() {
+    'static: loop { //~ ERROR invalid label name `'static`
+        break 'static //~ ERROR invalid label name `'static`
+    }
+}
diff --git a/src/test/compile-fail/lifetime-underscore.rs b/src/test/compile-fail/lifetime-underscore.rs
new file mode 100644
index 00000000000..102d3576e54
--- /dev/null
+++ b/src/test/compile-fail/lifetime-underscore.rs
@@ -0,0 +1,27 @@
+// Copyright 2016 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.
+
+#![deny(lifetime_underscore)]
+
+fn _f<'_>() //~ ERROR invalid lifetime name `'_`
+//~^ WARN this was previously accepted
+    -> &'_ u8 //~ ERROR invalid lifetime name `'_`
+    //~^ WARN this was previously accepted
+{
+    panic!();
+}
+
+fn main() {
+    '_: loop { //~ ERROR invalid label name `'_`
+    //~^ WARN this was previously accepted
+        break '_ //~ ERROR invalid label name `'_`
+        //~^ WARN this was previously accepted
+    }
+}
diff --git a/src/test/compile-fail/priv-in-bad-locations.rs b/src/test/compile-fail/priv-in-bad-locations.rs
index 43d112b8aa0..8901d8c08e5 100644
--- a/src/test/compile-fail/priv-in-bad-locations.rs
+++ b/src/test/compile-fail/priv-in-bad-locations.rs
@@ -8,8 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-pub extern {
-    //~^ ERROR unnecessary visibility
+pub extern { //~ ERROR unnecessary visibility qualifier
     pub fn bar();
 }
 
@@ -19,10 +18,10 @@ trait A {
 
 struct B;
 
-pub impl B {} //~ ERROR: unnecessary visibility
+pub impl B {} //~ ERROR unnecessary visibility qualifier
 
-pub impl A for B { //~ ERROR: unnecessary visibility
-    pub fn foo(&self) {} //~ ERROR: unnecessary visibility
+pub impl A for B { //~ ERROR unnecessary visibility qualifier
+    pub fn foo(&self) {} //~ ERROR unnecessary visibility qualifier
 }
 
 pub fn main() {}
diff --git a/src/test/compile-fail/privacy/restricted/ty-params.rs b/src/test/compile-fail/privacy/restricted/ty-params.rs
index ab423620d68..ae60c4366ee 100644
--- a/src/test/compile-fail/privacy/restricted/ty-params.rs
+++ b/src/test/compile-fail/privacy/restricted/ty-params.rs
@@ -16,9 +16,11 @@ macro_rules! m {
 
 struct S<T>(T);
 m!{ S<u8> } //~ ERROR type or lifetime parameters in visibility path
+//~^ ERROR failed to resolve module path. Not a module `S`
 
 mod foo {
     struct S(pub(foo<T>) ()); //~ ERROR type or lifetime parameters in visibility path
+    //~^ ERROR type name `T` is undefined or not in scope
 }
 
 fn main() {}
diff --git a/src/test/compile-fail/use-super-global-path.rs b/src/test/compile-fail/use-super-global-path.rs
index d721d428f29..1d0d60a775f 100644
--- a/src/test/compile-fail/use-super-global-path.rs
+++ b/src/test/compile-fail/use-super-global-path.rs
@@ -9,10 +9,17 @@
 // except according to those terms.
 
 #![feature(rustc_attrs)]
+#![allow(unused_imports, dead_code)]
+
+struct S;
+struct Z;
 
 mod foo {
+    use ::super::{S, Z}; //~ WARN global paths cannot start with `super`
+    //~^ WARN this was previously accepted by the compiler but is being phased out
+
     pub fn g() {
-        use ::super::main; //~ WARN expected identifier, found keyword `super`
+        use ::super::main; //~ WARN global paths cannot start with `super`
         //~^ WARN this was previously accepted by the compiler but is being phased out
         main();
     }
diff --git a/src/test/compile-fail/useless-pub.rs b/src/test/compile-fail/useless-pub.rs
index 268b937c291..064062df753 100644
--- a/src/test/compile-fail/useless-pub.rs
+++ b/src/test/compile-fail/useless-pub.rs
@@ -15,14 +15,12 @@ pub trait E {
 }
 
 impl E for A {
-    pub fn foo(&self) {}             //~ ERROR: unnecessary visibility
+    pub fn foo(&self) {} //~ ERROR: unnecessary visibility qualifier
 }
 
 enum Foo {
     V1 { pub f: i32 }, //~ ERROR unnecessary visibility qualifier
-                       //| NOTE visibility qualifiers have no effect on variant fields
     V2(pub i32), //~ ERROR unnecessary visibility qualifier
-                 //| NOTE visibility qualifiers have no effect on variant fields
 }
 
 fn main() {}