From 2abdf963441dad5e5ec516af8ee2f9b459f9e47d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 22 May 2016 17:51:22 +0300 Subject: [PATCH 1/3] Add an AST sanity checking pass and use it to catch some illegal lifetime/label names --- src/librustc/lint/builtin.rs | 9 ++- src/librustc_driver/driver.rs | 6 +- src/librustc_lint/lib.rs | 4 + src/librustc_passes/ast_sanity.rs | 79 ++++++++++++++++++++ src/librustc_passes/lib.rs | 1 + src/test/compile-fail/label-static.rs | 15 ++++ src/test/compile-fail/lifetime-underscore.rs | 27 +++++++ 7 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 src/librustc_passes/ast_sanity.rs create mode 100644 src/test/compile-fail/label-static.rs create mode 100644 src/test/compile-fail/lifetime-underscore.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index d7971cd2cf040..41086b5d1c990 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 48b86d862f575..570135e071378 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_sanity, 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 sanity checking", + || ast_sanity::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 9fca6d3d20139..ed12d0d9f3c11 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 ", }, + FutureIncompatibleInfo { + id: LintId::of(LIFETIME_UNDERSCORE), + reference: "RFC 1177 ", + }, ]); // We have one lint pass defined specially diff --git a/src/librustc_passes/ast_sanity.rs b/src/librustc_passes/ast_sanity.rs new file mode 100644 index 0000000000000..22f73896f099f --- /dev/null +++ b/src/librustc_passes/ast_sanity.rs @@ -0,0 +1,79 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Sanity check 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::keywords; +use syntax::visit::{self, Visitor}; + +struct SanityChecker<'a> { + session: &'a Session, +} + +impl<'a> SanityChecker<'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) + ); + } + } +} + +impl<'a, 'v> Visitor<'v> for SanityChecker<'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)) => { + self.check_label(ident, expr.span, expr.id); + } + ExprKind::Break(Some(ident)) | ExprKind::Again(Some(ident)) => { + self.check_label(ident.node, ident.span, expr.id); + } + _ => {} + } + + visit::walk_expr(self, expr) + } +} + +pub fn check_crate(session: &Session, krate: &Crate) { + visit::walk_crate(&mut SanityChecker { session: session }, krate) +} diff --git a/src/librustc_passes/lib.rs b/src/librustc_passes/lib.rs index 67a9c2fd17e9f..9e5cc13904097 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_sanity; pub mod consts; pub mod loops; pub mod no_asm; diff --git a/src/test/compile-fail/label-static.rs b/src/test/compile-fail/label-static.rs new file mode 100644 index 0000000000000..a0fb25ea06eac --- /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 or the MIT license +// , 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 0000000000000..102d3576e5467 --- /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 or the MIT license +// , 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 + } +} From c02c6e88e67e7c23a037f90569cc0072c37d12df Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 22 May 2016 18:07:28 +0300 Subject: [PATCH 2/3] Move some other checks to AST sanity pass --- src/librustc_passes/ast_sanity.rs | 96 ++++++++++++++++- src/librustc_passes/diagnostics.rs | 40 +++++++ src/librustc_privacy/Cargo.toml | 1 - src/librustc_privacy/diagnostics.rs | 40 ------- src/librustc_privacy/lib.rs | 102 ++---------------- src/librustc_resolve/build_reduced_graph.rs | 39 +------ src/libsyntax/ast.rs | 10 ++ src/libsyntax/feature_gate.rs | 32 +----- src/test/compile-fail/issue-12560-1.rs | 2 +- src/test/compile-fail/issue-29161.rs | 2 +- .../compile-fail/priv-in-bad-locations.rs | 9 +- .../privacy/restricted/ty-params.rs | 2 + .../compile-fail/use-super-global-path.rs | 2 +- src/test/compile-fail/useless-pub.rs | 4 +- 14 files changed, 169 insertions(+), 212 deletions(-) diff --git a/src/librustc_passes/ast_sanity.rs b/src/librustc_passes/ast_sanity.rs index 22f73896f099f..e22b3dfb81c52 100644 --- a/src/librustc_passes/ast_sanity.rs +++ b/src/librustc_passes/ast_sanity.rs @@ -21,7 +21,7 @@ use rustc::session::Session; use syntax::ast::*; use syntax::codemap::Span; use syntax::errors; -use syntax::parse::token::keywords; +use syntax::parse::token::{self, keywords}; use syntax::visit::{self, Visitor}; struct SanityChecker<'a> { @@ -44,6 +44,17 @@ impl<'a> SanityChecker<'a> { ); } } + + 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 SanityChecker<'a> { @@ -72,6 +83,89 @@ impl<'a, 'v> Visitor<'v> for SanityChecker<'a> { 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) { diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs index 77f896e011b93..ebbbc89e57e23 100644 --- a/src/librustc_passes/diagnostics.rs +++ b/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! { diff --git a/src/librustc_privacy/Cargo.toml b/src/librustc_privacy/Cargo.toml index 0553e54e3aa9b..ac33c23f023d8 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 1b49409970ded..bc84827ddf428 100644 --- a/src/librustc_privacy/diagnostics.rs +++ b/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: diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index c90d152e3c314..7e76842a9f4a7 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 e0243bf4fa690..c7b113689fde9 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 c8ded115db864..cd735be9fdf18 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1907,6 +1907,16 @@ pub enum ViewPath_ { ViewPathList(Path, Vec) } +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; diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 5687099b27ced..ed9c49445a61e 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -952,22 +952,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") { @@ -1138,22 +1122,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/test/compile-fail/issue-12560-1.rs b/src/test/compile-fail/issue-12560-1.rs index e005de01649a5..80f551ebd1f7c 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 f7453c45be645..bc09f61a754c2 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/priv-in-bad-locations.rs b/src/test/compile-fail/priv-in-bad-locations.rs index 43d112b8aa004..8901d8c08e50c 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 ab423620d6866..ae60c4366ee33 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); m!{ S } //~ ERROR type or lifetime parameters in visibility path +//~^ ERROR failed to resolve module path. Not a module `S` mod foo { struct S(pub(foo) ()); //~ 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 d721d428f29f5..c4d18a94eb2dc 100644 --- a/src/test/compile-fail/use-super-global-path.rs +++ b/src/test/compile-fail/use-super-global-path.rs @@ -12,7 +12,7 @@ mod foo { 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 268b937c29165..064062df753b6 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() {} From 731144b95a1040ca692296d064c5be47fa927d3c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 6 Mar 2016 15:54:44 +0300 Subject: [PATCH 3/3] sanity -> validation Add test for `::super` in import prefix --- src/librustc_driver/driver.rs | 6 +++--- .../{ast_sanity.rs => ast_validation.rs} | 14 ++++++-------- src/librustc_passes/lib.rs | 2 +- src/rustc/Cargo.lock | 1 - src/test/compile-fail/use-super-global-path.rs | 7 +++++++ 5 files changed, 17 insertions(+), 13 deletions(-) rename src/librustc_passes/{ast_sanity.rs => ast_validation.rs} (95%) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 570135e071378..95be6d5b62301 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::{ast_sanity, 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; @@ -167,8 +167,8 @@ pub fn compile_input(sess: &Session, || lint::check_ast_crate(sess, &expanded_crate)); time(sess.time_passes(), - "AST sanity checking", - || ast_sanity::check_crate(sess, &expanded_crate)); + "AST validation", + || ast_validation::check_crate(sess, &expanded_crate)); let (analysis, resolutions, mut hir_forest) = { lower_and_resolve(sess, &id, &mut defs, &expanded_crate, diff --git a/src/librustc_passes/ast_sanity.rs b/src/librustc_passes/ast_validation.rs similarity index 95% rename from src/librustc_passes/ast_sanity.rs rename to src/librustc_passes/ast_validation.rs index e22b3dfb81c52..919c717f888ff 100644 --- a/src/librustc_passes/ast_sanity.rs +++ b/src/librustc_passes/ast_validation.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// Sanity check AST before lowering it to HIR +// 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, @@ -24,11 +24,11 @@ use syntax::errors; use syntax::parse::token::{self, keywords}; use syntax::visit::{self, Visitor}; -struct SanityChecker<'a> { +struct AstValidator<'a> { session: &'a Session, } -impl<'a> SanityChecker<'a> { +impl<'a> AstValidator<'a> { fn err_handler(&self) -> &errors::Handler { &self.session.parse_sess.span_diagnostic } @@ -57,7 +57,7 @@ impl<'a> SanityChecker<'a> { } } -impl<'a, 'v> Visitor<'v> for SanityChecker<'a> { +impl<'a, 'v> Visitor<'v> for AstValidator<'a> { fn visit_lifetime(&mut self, lt: &Lifetime) { if lt.name.as_str() == "'_" { self.session.add_lint( @@ -72,9 +72,7 @@ impl<'a, 'v> Visitor<'v> for SanityChecker<'a> { 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)) => { - self.check_label(ident, expr.span, expr.id); - } + 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); } @@ -169,5 +167,5 @@ impl<'a, 'v> Visitor<'v> for SanityChecker<'a> { } pub fn check_crate(session: &Session, krate: &Crate) { - visit::walk_crate(&mut SanityChecker { session: session }, krate) + visit::walk_crate(&mut AstValidator { session: session }, krate) } diff --git a/src/librustc_passes/lib.rs b/src/librustc_passes/lib.rs index 9e5cc13904097..1576ca6bdeaa4 100644 --- a/src/librustc_passes/lib.rs +++ b/src/librustc_passes/lib.rs @@ -37,7 +37,7 @@ extern crate rustc_const_math; pub mod diagnostics; -pub mod ast_sanity; +pub mod ast_validation; pub mod consts; pub mod loops; pub mod no_asm; diff --git a/src/rustc/Cargo.lock b/src/rustc/Cargo.lock index 7a63742fba342..931f90a239421 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/use-super-global-path.rs b/src/test/compile-fail/use-super-global-path.rs index c4d18a94eb2dc..1d0d60a775fda 100644 --- a/src/test/compile-fail/use-super-global-path.rs +++ b/src/test/compile-fail/use-super-global-path.rs @@ -9,8 +9,15 @@ // 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 global paths cannot start with `super` //~^ WARN this was previously accepted by the compiler but is being phased out