Add AST validation pass and move some checks to it

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
         )
     }
 }

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;
 
@@ -166,6 +166,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)

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

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)
+}

src/librustc_passes/diagnostics.rs

@@ -118,6 +118,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! {

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;

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" }

src/librustc_privacy/diagnostics.rs

@@ -111,46 +111,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: