Auto merge of #47799 - topecongiro:fix-span-of-visibility, r=petrochenkov

Fix span of visibility

This PR

1. adds a closing parenthesis to the span of `Visibility::Crate` (e.g. `pub(crate)`). The current span only covers `pub(crate`.
2. adds a `span` field to `Visibility::Restricted`. This span covers the entire visibility expression (e.g. `pub (in self)`). Currently all we can have is a span for `Path`.

This PR is motivated by the bug found in rustfmt (https://github.com/rust-lang-nursery/rustfmt/issues/2398).

The first change is a strict improvement IMHO. The second change may not be desirable, as it adds a field which is currently not used by the compiler.

26 files changed, 174 insertions, 129 deletions

diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 55dcb16c3c9..e3af2850538 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -3362,10 +3362,10 @@ impl<'a> LoweringContext<'a> {
                         v: &Visibility,
                         explicit_owner: Option<NodeId>)
                         -> hir::Visibility {
-        match *v {
-            Visibility::Public => hir::Public,
-            Visibility::Crate(..) => hir::Visibility::Crate,
-            Visibility::Restricted { ref path, id } => {
+        match v.node {
+            VisibilityKind::Public => hir::Public,
+            VisibilityKind::Crate(..) => hir::Visibility::Crate,
+            VisibilityKind::Restricted { ref path, id, .. } => {
                 hir::Visibility::Restricted {
                     path: P(self.lower_path(id, path, ParamMode::Explicit, true)),
                     id: if let Some(owner) = explicit_owner {
@@ -3375,7 +3375,7 @@ impl<'a> LoweringContext<'a> {
                     }
                 }
             }
-            Visibility::Inherited => hir::Inherited,
+            VisibilityKind::Inherited => hir::Inherited,
         }
     }
 
diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs
index 352184c1efa..c088458c355 100644
--- a/src/librustc_allocator/expand.rs
+++ b/src/librustc_allocator/expand.rs
@@ -13,9 +13,9 @@ use rustc_errors;
 use syntax::abi::Abi;
 use syntax::ast::{Crate, Attribute, LitKind, StrStyle, ExprKind};
 use syntax::ast::{Unsafety, Constness, Generics, Mutability, Ty, Mac, Arg};
-use syntax::ast::{self, Ident, Item, ItemKind, TyKind, Visibility, Expr};
+use syntax::ast::{self, Ident, Item, ItemKind, TyKind, VisibilityKind, Expr};
 use syntax::attr;
-use syntax::codemap::dummy_spanned;
+use syntax::codemap::{dummy_spanned, respan};
 use syntax::codemap::{ExpnInfo, NameAndSpan, MacroAttribute};
 use syntax::ext::base::ExtCtxt;
 use syntax::ext::base::Resolver;
@@ -97,7 +97,11 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> {
         ]);
         let mut items = vec![
             f.cx.item_extern_crate(f.span, f.alloc),
-            f.cx.item_use_simple(f.span, Visibility::Inherited, super_path),
+            f.cx.item_use_simple(
+                f.span,
+                respan(f.span.empty(), VisibilityKind::Inherited),
+                super_path,
+            ),
         ];
         for method in ALLOCATOR_METHODS {
             items.push(f.allocator_fn(method));
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
index d73e968a827..c1340d0a28a 100644
--- a/src/librustc_metadata/cstore_impl.rs
+++ b/src/librustc_metadata/cstore_impl.rs
@@ -34,6 +34,7 @@ use std::rc::Rc;
 
 use syntax::ast;
 use syntax::attr;
+use syntax::codemap;
 use syntax::ext::base::SyntaxExtension;
 use syntax::parse::filemap_to_stream;
 use syntax::symbol::Symbol;
@@ -496,7 +497,7 @@ impl CrateStore for cstore::CStore {
                 tokens: body.into(),
                 legacy: def.legacy,
             }),
-            vis: ast::Visibility::Inherited,
+            vis: codemap::respan(local_span.empty(), ast::VisibilityKind::Inherited),
             tokens: None,
         })
     }
diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
index 6971033c899..bb6dbe632e3 100644
--- a/src/librustc_passes/ast_validation.rs
+++ b/src/librustc_passes/ast_validation.rs
@@ -58,14 +58,14 @@ impl<'a> AstValidator<'a> {
         }
     }
 
-    fn invalid_visibility(&self, vis: &Visibility, span: Span, note: Option<&str>) {
-        if vis != &Visibility::Inherited {
+    fn invalid_visibility(&self, vis: &Visibility, note: Option<&str>) {
+        if vis.node != VisibilityKind::Inherited {
             let mut err = struct_span_err!(self.session,
-                                           span,
+                                           vis.span,
                                            E0449,
                                            "unnecessary visibility qualifier");
-            if vis == &Visibility::Public {
-                err.span_label(span, "`pub` not needed here");
+            if vis.node == VisibilityKind::Public {
+                err.span_label(vis.span, "`pub` not needed here");
             }
             if let Some(note) = note {
                 err.note(note);
@@ -216,7 +216,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     fn visit_item(&mut self, item: &'a Item) {
         match item.node {
             ItemKind::Impl(unsafety, polarity, _, _, Some(..), ref ty, ref impl_items) => {
-                self.invalid_visibility(&item.vis, item.span, None);
+                self.invalid_visibility(&item.vis, None);
                 if ty.node == TyKind::Err {
                     self.err_handler()
                         .struct_span_err(item.span, "`impl Trait for .. {}` is an obsolete syntax")
@@ -226,7 +226,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     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);
+                    self.invalid_visibility(&impl_item.vis, None);
                     if let ImplItemKind::Method(ref sig, _) = impl_item.node {
                         self.check_trait_fn_not_const(sig.constness);
                     }
@@ -234,7 +234,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             }
             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");
@@ -247,16 +246,16 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 }
             }
             ItemKind::ForeignMod(..) => {
-                self.invalid_visibility(&item.vis,
-                                        item.span,
-                                        Some("place qualifiers on individual foreign items \
-                                              instead"));
+                self.invalid_visibility(
+                    &item.vis,
+                    Some("place qualifiers on individual foreign items instead"),
+                );
             }
             ItemKind::Enum(ref def, _) => {
                 for variant in &def.variants {
                     self.invalid_non_exhaustive_attribute(variant);
                     for field in variant.node.data.fields() {
-                        self.invalid_visibility(&field.vis, field.span, None);
+                        self.invalid_visibility(&field.vis, None);
                     }
                 }
             }
@@ -359,8 +358,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     }
 
     fn visit_vis(&mut self, vis: &'a Visibility) {
-        match *vis {
-            Visibility::Restricted { ref path, .. } => {
+        match vis.node {
+            VisibilityKind::Restricted { ref path, .. } => {
                 path.segments.iter().find(|segment| segment.parameters.is_some()).map(|segment| {
                     self.err_handler().span_err(segment.parameters.as_ref().unwrap().span(),
                                                 "generic arguments in visibility path");
diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs
index a757ac92df5..163f6a64010 100644
--- a/src/librustc_resolve/check_unused.rs
+++ b/src/librustc_resolve/check_unused.rs
@@ -86,7 +86,7 @@ impl<'a, 'b> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b> {
         // because this means that they were generated in some fashion by the
         // compiler and we don't need to consider them.
         if let ast::ItemKind::Use(..) = item.node {
-            if item.vis == ast::Visibility::Public || item.span.source_equal(&DUMMY_SP) {
+            if item.vis.node == ast::VisibilityKind::Public || item.span.source_equal(&DUMMY_SP) {
                 return;
             }
         }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index a9ba278ea74..e6b9150fa3a 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -3796,13 +3796,15 @@ impl<'a> Resolver<'a> {
     }
 
     fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility {
-        match *vis {
-            ast::Visibility::Public => ty::Visibility::Public,
-            ast::Visibility::Crate(..) => ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
-            ast::Visibility::Inherited => {
+        match vis.node {
+            ast::VisibilityKind::Public => ty::Visibility::Public,
+            ast::VisibilityKind::Crate(..) => {
+                ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))
+            }
+            ast::VisibilityKind::Inherited => {
                 ty::Visibility::Restricted(self.current_module.normal_ancestor_id)
             }
-            ast::Visibility::Restricted { ref path, id } => {
+            ast::VisibilityKind::Restricted { ref path, id, .. } => {
                 let def = self.smart_resolve_path(id, None, path,
                                                   PathSource::Visibility).base_def();
                 if def == Def::Err {
diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
index 47530c42085..bf82b077423 100644
--- a/src/librustc_save_analysis/dump_visitor.rs
+++ b/src/librustc_save_analysis/dump_visitor.rs
@@ -43,7 +43,7 @@ use syntax::print::pprust::{
     ty_to_string
 };
 use syntax::ptr::P;
-use syntax::codemap::{Spanned, DUMMY_SP};
+use syntax::codemap::{Spanned, DUMMY_SP, respan};
 use syntax_pos::*;
 
 use {escape, generated_code, lower_attributes, PathCollector, SaveContext};
@@ -65,12 +65,19 @@ macro_rules! down_cast_data {
 }
 
 macro_rules! access_from {
+    ($save_ctxt:expr, $vis:expr, $id:expr) => {
+        Access {
+            public: $vis.node == ast::VisibilityKind::Public,
+            reachable: $save_ctxt.analysis.access_levels.is_reachable($id),
+        }
+    };
+
     ($save_ctxt:expr, $item:expr) => {
         Access {
-            public: $item.vis == ast::Visibility::Public,
+            public: $item.vis.node == ast::VisibilityKind::Public,
             reachable: $save_ctxt.analysis.access_levels.is_reachable($item.id),
         }
-    }
+    };
 }
 
 pub struct DumpVisitor<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> {
@@ -405,12 +412,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
 
             method_data.value = sig_str;
             method_data.sig = sig::method_signature(id, name, generics, sig, &self.save_ctxt);
-            self.dumper.dump_def(
-                &Access {
-                    public: vis == ast::Visibility::Public,
-                    reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
-                },
-                method_data);
+            self.dumper.dump_def(&access_from!(self.save_ctxt, vis, id), method_data);
         }
 
         // walk arg and return types
@@ -543,10 +545,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
             let span = self.span_from_span(sub_span.expect("No span found for variable"));
 
             self.dumper.dump_def(
-                &Access {
-                    public: vis == ast::Visibility::Public,
-                    reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
-                },
+                &access_from!(self.save_ctxt, vis, id),
                 Def {
                     kind: DefKind::Const,
                     id: ::id_from_node_id(id, &self.save_ctxt),
@@ -597,7 +596,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
                     .iter()
                     .enumerate()
                     .filter_map(|(i, f)| {
-                        if include_priv_fields || f.vis == ast::Visibility::Public {
+                        if include_priv_fields || f.vis.node == ast::VisibilityKind::Public {
                             f.ident
                                 .map(|i| i.to_string())
                                 .or_else(|| Some(i.to_string()))
@@ -1135,6 +1134,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
 
     fn process_trait_item(&mut self, trait_item: &'l ast::TraitItem, trait_id: DefId) {
         self.process_macro_use(trait_item.span);
+        let vis_span = trait_item.span.empty();
         match trait_item.node {
             ast::TraitItemKind::Const(ref ty, ref expr) => {
                 self.process_assoc_const(
@@ -1144,7 +1144,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
                     &ty,
                     expr.as_ref().map(|e| &**e),
                     trait_id,
-                    ast::Visibility::Public,
+                    respan(vis_span, ast::VisibilityKind::Public),
                     &trait_item.attrs,
                 );
             }
@@ -1155,7 +1155,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
                     trait_item.id,
                     trait_item.ident,
                     &trait_item.generics,
-                    ast::Visibility::Public,
+                    respan(vis_span, ast::VisibilityKind::Public),
                     trait_item.span,
                 );
             }
@@ -1259,10 +1259,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
 
         // The access is calculated using the current tree ID, but with the root tree's visibility
         // (since nested trees don't have their own visibility).
-        let access = Access {
-            public: root_item.vis == ast::Visibility::Public,
-            reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
-        };
+        let access = access_from!(self.save_ctxt, root_item.vis, id);
 
         // The parent def id of a given use tree is always the enclosing item.
         let parent = self.save_ctxt.tcx.hir.opt_local_def_id(id)
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 8c1e5cf7586..c7ce7fffaa2 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -1937,10 +1937,12 @@ pub enum CrateSugar {
     JustCrate,
 }
 
+pub type Visibility = Spanned<VisibilityKind>;
+
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
-pub enum Visibility {
+pub enum VisibilityKind {
     Public,
-    Crate(Span, CrateSugar),
+    Crate(CrateSugar),
     Restricted { path: P<Path>, id: NodeId },
     Inherited,
 }
diff --git a/src/libsyntax/diagnostics/plugin.rs b/src/libsyntax/diagnostics/plugin.rs
index 5224f52c496..dd27dea4f0d 100644
--- a/src/libsyntax/diagnostics/plugin.rs
+++ b/src/libsyntax/diagnostics/plugin.rs
@@ -14,6 +14,7 @@ use std::env;
 
 use ast;
 use ast::{Ident, Name};
+use codemap;
 use syntax_pos::Span;
 use ext::base::{ExtCtxt, MacEager, MacResult};
 use ext::build::AstBuilder;
@@ -234,7 +235,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,
                 ty,
                 expr,
             ),
-            vis: ast::Visibility::Public,
+            vis: codemap::respan(span.empty(), ast::VisibilityKind::Public),
             span,
             tokens: None,
         })
diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs
index 2e6de96d65a..7681f55bd8c 100644
--- a/src/libsyntax/ext/build.rs
+++ b/src/libsyntax/ext/build.rs
@@ -987,7 +987,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
             attrs,
             id: ast::DUMMY_NODE_ID,
             node,
-            vis: ast::Visibility::Inherited,
+            vis: respan(span.empty(), ast::VisibilityKind::Inherited),
             span,
             tokens: None,
         })
@@ -1033,7 +1033,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
                 span: ty.span,
                 ty,
                 ident: None,
-                vis: ast::Visibility::Inherited,
+                vis: respan(span.empty(), ast::VisibilityKind::Inherited),
                 attrs: Vec::new(),
                 id: ast::DUMMY_NODE_ID,
             }
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 44a073545a7..d4d9dfb01da 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -11,7 +11,7 @@
 use ast::{self, Block, Ident, NodeId, PatKind, Path};
 use ast::{MacStmtStyle, StmtKind, ItemKind};
 use attr::{self, HasAttrs};
-use codemap::{ExpnInfo, NameAndSpan, MacroBang, MacroAttribute, dummy_spanned};
+use codemap::{ExpnInfo, NameAndSpan, MacroBang, MacroAttribute, dummy_spanned, respan};
 use config::{is_test_or_bench, StripUnconfigured};
 use errors::FatalError;
 use ext::base::*;
@@ -238,7 +238,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             node: ast::ItemKind::Mod(krate.module),
             ident: keywords::Invalid.ident(),
             id: ast::DUMMY_NODE_ID,
-            vis: ast::Visibility::Public,
+            vis: respan(krate.span.empty(), ast::VisibilityKind::Public),
             tokens: None,
         })));
 
@@ -1022,7 +1022,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
             // Ensure that test functions are accessible from the test harness.
             ast::ItemKind::Fn(..) if self.cx.ecfg.should_test => {
                 if item.attrs.iter().any(|attr| is_test_or_bench(attr)) {
-                    item = item.map(|mut item| { item.vis = ast::Visibility::Public; item });
+                    item = item.map(|mut item| {
+                        item.vis = respan(item.vis.span, ast::VisibilityKind::Public);
+                        item
+                    });
                 }
                 noop_fold_item(item, self)
             }
diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs
index 2f5b386346b..9c2c22476e9 100644
--- a/src/libsyntax/ext/placeholders.rs
+++ b/src/libsyntax/ext/placeholders.rs
@@ -33,7 +33,7 @@ pub fn placeholder(kind: ExpansionKind, id: ast::NodeId) -> Expansion {
     let ident = keywords::Invalid.ident();
     let attrs = Vec::new();
     let generics = ast::Generics::default();
-    let vis = ast::Visibility::Inherited;
+    let vis = dummy_spanned(ast::VisibilityKind::Inherited);
     let span = DUMMY_SP;
     let expr_placeholder = || P(ast::Expr {
         id, span,
diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs
index 7fcd88c94ca..7a024dbad88 100644
--- a/src/libsyntax/ext/quote.rs
+++ b/src/libsyntax/ext/quote.rs
@@ -9,6 +9,7 @@
 // except according to those terms.
 
 use ast::{self, Arg, Arm, Block, Expr, Item, Pat, Stmt, Ty};
+use codemap::respan;
 use syntax_pos::Span;
 use ext::base::ExtCtxt;
 use ext::base;
@@ -855,7 +856,12 @@ fn expand_wrapper(cx: &ExtCtxt,
     let mut stmts = imports.iter().map(|path| {
         // make item: `use ...;`
         let path = path.iter().map(|s| s.to_string()).collect();
-        cx.stmt_item(sp, cx.item_use_glob(sp, ast::Visibility::Inherited, ids_ext(path)))
+        let use_item = cx.item_use_glob(
+            sp,
+            respan(sp.empty(), ast::VisibilityKind::Inherited),
+            ids_ext(path),
+        );
+        cx.stmt_item(sp, use_item)
     }).chain(Some(stmt_let_ext_cx)).collect::<Vec<_>>();
     stmts.push(cx.stmt_expr(expr));
 
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
index 3b137f9570a..c0fde71d086 100644
--- a/src/libsyntax/feature_gate.rs
+++ b/src/libsyntax/feature_gate.rs
@@ -1816,8 +1816,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
     }
 
     fn visit_vis(&mut self, vis: &'a ast::Visibility) {
-        if let ast::Visibility::Crate(span, ast::CrateSugar::JustCrate) = *vis {
-            gate_feature_post!(&self, crate_visibility_modifier, span,
+        if let ast::VisibilityKind::Crate(ast::CrateSugar::JustCrate) = vis.node {
+            gate_feature_post!(&self, crate_visibility_modifier, vis.span,
                                "`crate` visibility modifier is experimental");
         }
         visit::walk_vis(self, vis);
diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
index 921ed3565a4..1a2025b073b 100644
--- a/src/libsyntax/fold.rs
+++ b/src/libsyntax/fold.rs
@@ -1018,7 +1018,7 @@ pub fn noop_fold_crate<T: Folder>(Crate {module, attrs, span}: Crate,
         ident: keywords::Invalid.ident(),
         attrs,
         id: ast::DUMMY_NODE_ID,
-        vis: ast::Visibility::Public,
+        vis: respan(span.empty(), ast::VisibilityKind::Public),
         span,
         node: ast::ItemKind::Mod(module),
         tokens: None,
@@ -1367,11 +1367,13 @@ pub fn noop_fold_stmt_kind<T: Folder>(node: StmtKind, folder: &mut T) -> SmallVe
 }
 
 pub fn noop_fold_vis<T: Folder>(vis: Visibility, folder: &mut T) -> Visibility {
-    match vis {
-        Visibility::Restricted { path, id } => Visibility::Restricted {
-            path: path.map(|path| folder.fold_path(path)),
-            id: folder.new_id(id)
-        },
+    match vis.node {
+        VisibilityKind::Restricted { path, id } => {
+            respan(vis.span, VisibilityKind::Restricted {
+                path: path.map(|path| folder.fold_path(path)),
+                id: folder.new_id(id),
+            })
+        }
         _ => vis,
     }
 }
diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs
index b671f81c2a8..06eb64e157c 100644
--- a/src/libsyntax/parse/mod.rs
+++ b/src/libsyntax/parse/mod.rs
@@ -664,7 +664,7 @@ pub fn integer_lit(s: &str, suffix: Option<Symbol>, diag: Option<(Span, &Handler
 mod tests {
     use super::*;
     use syntax_pos::{self, Span, BytePos, Pos, NO_EXPANSION};
-    use codemap::Spanned;
+    use codemap::{respan, Spanned};
     use ast::{self, Ident, PatKind};
     use abi::Abi;
     use attr::first_attr_value_str_by_name;
@@ -932,7 +932,7 @@ mod tests {
                                         span: sp(15,21),
                                         recovered: false,
                                     })),
-                            vis: ast::Visibility::Inherited,
+                            vis: respan(sp(0, 0), ast::VisibilityKind::Inherited),
                             span: sp(0,21)})));
     }
 
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 7915109ce3a..74daa5179d3 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -36,7 +36,7 @@ use ast::StrStyle;
 use ast::SelfKind;
 use ast::{TraitItem, TraitRef, TraitObjectSyntax};
 use ast::{Ty, TyKind, TypeBinding, TyParam, TyParamBounds};
-use ast::{Visibility, WhereClause, CrateSugar};
+use ast::{Visibility, VisibilityKind, WhereClause, CrateSugar};
 use ast::{UseTree, UseTreeKind};
 use ast::{BinOpKind, UnOp};
 use ast::{RangeEnd, RangeSyntax};
@@ -4132,7 +4132,7 @@ impl<'a> Parser<'a> {
             token::Ident(ident) if ident.name == "macro_rules" &&
                                    self.look_ahead(1, |t| *t == token::Not) => {
                 let prev_span = self.prev_span;
-                self.complain_if_pub_macro(vis, prev_span);
+                self.complain_if_pub_macro(&vis.node, prev_span);
                 self.bump();
                 self.bump();
 
@@ -4169,7 +4169,11 @@ impl<'a> Parser<'a> {
                 node: StmtKind::Local(self.parse_local(attrs.into())?),
                 span: lo.to(self.prev_span),
             }
-        } else if let Some(macro_def) = self.eat_macro_def(&attrs, &Visibility::Inherited, lo)? {
+        } else if let Some(macro_def) = self.eat_macro_def(
+            &attrs,
+            &codemap::respan(lo, VisibilityKind::Inherited),
+            lo,
+        )? {
             Stmt {
                 id: ast::DUMMY_NODE_ID,
                 node: StmtKind::Item(macro_def),
@@ -4296,7 +4300,7 @@ impl<'a> Parser<'a> {
                         self.mk_item(
                             span, id /*id is good here*/,
                             ItemKind::Mac(respan(span, Mac_ { path: pth, tts: tts })),
-                            Visibility::Inherited,
+                            respan(lo, VisibilityKind::Inherited),
                             attrs)
                     }),
                 }
@@ -5213,15 +5217,15 @@ impl<'a> Parser<'a> {
         })
     }
 
-    fn complain_if_pub_macro(&mut self, vis: &Visibility, sp: Span) {
+    fn complain_if_pub_macro(&mut self, vis: &VisibilityKind, sp: Span) {
         if let Err(mut err) = self.complain_if_pub_macro_diag(vis, sp) {
             err.emit();
         }
     }
 
-    fn complain_if_pub_macro_diag(&mut self, vis: &Visibility, sp: Span) -> PResult<'a, ()> {
+    fn complain_if_pub_macro_diag(&mut self, vis: &VisibilityKind, sp: Span) -> PResult<'a, ()> {
         match *vis {
-            Visibility::Inherited => Ok(()),
+            VisibilityKind::Inherited => Ok(()),
             _ => {
                 let is_macro_rules: bool = match self.token {
                     token::Ident(sid) => sid.name == Symbol::intern("macro_rules"),
@@ -5283,7 +5287,7 @@ impl<'a> Parser<'a> {
                 self.expect(&token::Not)?;
             }
 
-            self.complain_if_pub_macro(vis, prev_span);
+            self.complain_if_pub_macro(&vis.node, prev_span);
 
             // eat a matched-delimiter token tree:
             *at_end = true;
@@ -5686,12 +5690,13 @@ impl<'a> Parser<'a> {
         self.expected_tokens.push(TokenType::Keyword(keywords::Crate));
         if self.is_crate_vis() {
             self.bump(); // `crate`
-            return Ok(Visibility::Crate(self.prev_span, CrateSugar::JustCrate));
+            return Ok(respan(self.prev_span, VisibilityKind::Crate(CrateSugar::JustCrate)));
         }
 
         if !self.eat_keyword(keywords::Pub) {
-            return Ok(Visibility::Inherited)
+            return Ok(respan(self.prev_span, VisibilityKind::Inherited))
         }
+        let lo = self.prev_span;
 
         if self.check(&token::OpenDelim(token::Paren)) {
             // We don't `self.bump()` the `(` yet because this might be a struct definition where
@@ -5702,25 +5707,35 @@ impl<'a> Parser<'a> {
                 // `pub(crate)`
                 self.bump(); // `(`
                 self.bump(); // `crate`
-                let vis = Visibility::Crate(self.prev_span, CrateSugar::PubCrate);
                 self.expect(&token::CloseDelim(token::Paren))?; // `)`
+                let vis = respan(
+                    lo.to(self.prev_span),
+                    VisibilityKind::Crate(CrateSugar::PubCrate),
+                );
                 return Ok(vis)
             } else if self.look_ahead(1, |t| t.is_keyword(keywords::In)) {
                 // `pub(in path)`
                 self.bump(); // `(`
                 self.bump(); // `in`
                 let path = self.parse_path(PathStyle::Mod)?.default_to_global(); // `path`
-                let vis = Visibility::Restricted { path: P(path), id: ast::DUMMY_NODE_ID };
                 self.expect(&token::CloseDelim(token::Paren))?; // `)`
+                let vis = respan(lo.to(self.prev_span), VisibilityKind::Restricted {
+                    path: P(path),
+                    id: ast::DUMMY_NODE_ID,
+                });
                 return Ok(vis)
             } else if self.look_ahead(2, |t| t == &token::CloseDelim(token::Paren)) &&
                       self.look_ahead(1, |t| t.is_keyword(keywords::Super) ||
-                                             t.is_keyword(keywords::SelfValue)) {
+                                             t.is_keyword(keywords::SelfValue))
+            {
                 // `pub(self)` or `pub(super)`
                 self.bump(); // `(`
                 let path = self.parse_path(PathStyle::Mod)?.default_to_global(); // `super`/`self`
-                let vis = Visibility::Restricted { path: P(path), id: ast::DUMMY_NODE_ID };
                 self.expect(&token::CloseDelim(token::Paren))?; // `)`
+                let vis = respan(lo.to(self.prev_span), VisibilityKind::Restricted {
+                    path: P(path),
+                    id: ast::DUMMY_NODE_ID,
+                });
                 return Ok(vis)
             } else if !can_take_tuple {  // Provide this diagnostic if this is not a tuple struct
                 // `pub(something) fn ...` or `struct X { pub(something) y: Z }`
@@ -5740,7 +5755,7 @@ impl<'a> Parser<'a> {
             }
         }
 
-        Ok(Visibility::Public)
+        Ok(respan(lo, VisibilityKind::Public))
     }
 
     /// Parse defaultness: `default` or nothing.
@@ -6573,7 +6588,7 @@ impl<'a> Parser<'a> {
 
         // Verify whether we have encountered a struct or method definition where the user forgot to
         // add the `struct` or `fn` keyword after writing `pub`: `pub S {}`
-        if visibility == Visibility::Public &&
+        if visibility.node == VisibilityKind::Public &&
             self.check_ident() &&
             self.look_ahead(1, |t| *t != token::Not)
         {
@@ -6681,7 +6696,7 @@ impl<'a> Parser<'a> {
             // MACRO INVOCATION ITEM
 
             let prev_span = self.prev_span;
-            self.complain_if_pub_macro(&visibility, prev_span);
+            self.complain_if_pub_macro(&visibility.node, prev_span);
 
             let mac_lo = self.span;
 
@@ -6715,8 +6730,8 @@ impl<'a> Parser<'a> {
         }
 
         // FAILURE TO PARSE ITEM
-        match visibility {
-            Visibility::Inherited => {}
+        match visibility.node {
+            VisibilityKind::Inherited => {}
             _ => {
                 return Err(self.span_fatal(self.prev_span, "unmatched visibility `pub`"));
             }
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index ae459c668aa..3dfe3c9e5b9 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -377,7 +377,7 @@ pub fn fun_to_string(decl: &ast::FnDecl,
     to_string(|s| {
         s.head("")?;
         s.print_fn(decl, unsafety, constness, Abi::Rust, Some(name),
-                   generics, &ast::Visibility::Inherited)?;
+                   generics, &codemap::dummy_spanned(ast::VisibilityKind::Inherited))?;
         s.end()?; // Close the head box
         s.end() // Close the outer box
     })
@@ -1458,13 +1458,13 @@ impl<'a> State<'a> {
     }
 
     pub fn print_visibility(&mut self, vis: &ast::Visibility) -> io::Result<()> {
-        match *vis {
-            ast::Visibility::Public => self.word_nbsp("pub"),
-            ast::Visibility::Crate(_, sugar) => match sugar {
+        match vis.node {
+            ast::VisibilityKind::Public => self.word_nbsp("pub"),
+            ast::VisibilityKind::Crate(sugar) => match sugar {
                 ast::CrateSugar::PubCrate => self.word_nbsp("pub(crate)"),
                 ast::CrateSugar::JustCrate => self.word_nbsp("crate")
             }
-            ast::Visibility::Restricted { ref path, .. } => {
+            ast::VisibilityKind::Restricted { ref path, .. } => {
                 let path = to_string(|s| s.print_path(path, false, 0, true));
                 if path == "self" || path == "super" {
                     self.word_nbsp(&format!("pub({})", path))
@@ -1472,7 +1472,7 @@ impl<'a> State<'a> {
                     self.word_nbsp(&format!("pub(in {})", path))
                 }
             }
-            ast::Visibility::Inherited => Ok(())
+            ast::VisibilityKind::Inherited => Ok(())
         }
     }
 
@@ -1569,15 +1569,23 @@ impl<'a> State<'a> {
         self.print_outer_attributes(&ti.attrs)?;
         match ti.node {
             ast::TraitItemKind::Const(ref ty, ref default) => {
-                self.print_associated_const(ti.ident, ty,
-                                            default.as_ref().map(|expr| &**expr),
-                                            &ast::Visibility::Inherited)?;
+                self.print_associated_const(
+                    ti.ident,
+                    ty,
+                    default.as_ref().map(|expr| &**expr),
+                    &codemap::respan(ti.span.empty(), ast::VisibilityKind::Inherited),
+                )?;
             }
             ast::TraitItemKind::Method(ref sig, ref body) => {
                 if body.is_some() {
                     self.head("")?;
                 }
-                self.print_method_sig(ti.ident, &ti.generics, sig, &ast::Visibility::Inherited)?;
+                self.print_method_sig(
+                    ti.ident,
+                    &ti.generics,
+                    sig,
+                    &codemap::respan(ti.span.empty(), ast::VisibilityKind::Inherited),
+                )?;
                 if let Some(ref body) = *body {
                     self.nbsp()?;
                     self.print_block_with_attrs(body, &ti.attrs)?;
@@ -3055,7 +3063,7 @@ impl<'a> State<'a> {
                       abi,
                       name,
                       &generics,
-                      &ast::Visibility::Inherited)?;
+                      &codemap::dummy_spanned(ast::VisibilityKind::Inherited))?;
         self.end()
     }
 
diff --git a/src/libsyntax/std_inject.rs b/src/libsyntax/std_inject.rs
index 00546400bb5..da24107f4c3 100644
--- a/src/libsyntax/std_inject.rs
+++ b/src/libsyntax/std_inject.rs
@@ -14,7 +14,7 @@ use std::cell::Cell;
 use ext::hygiene::{Mark, SyntaxContext};
 use symbol::{Symbol, keywords};
 use syntax_pos::{DUMMY_SP, Span};
-use codemap::{ExpnInfo, NameAndSpan, MacroAttribute};
+use codemap::{ExpnInfo, NameAndSpan, MacroAttribute, dummy_spanned, respan};
 use ptr::P;
 use tokenstream::TokenStream;
 
@@ -60,7 +60,7 @@ pub fn maybe_inject_crates_ref(mut krate: ast::Crate, alt_std_name: Option<Strin
         attrs: vec![attr::mk_attr_outer(DUMMY_SP,
                                         attr::mk_attr_id(),
                                         attr::mk_word_item(Symbol::intern("macro_use")))],
-        vis: ast::Visibility::Inherited,
+        vis: dummy_spanned(ast::VisibilityKind::Inherited),
         node: ast::ItemKind::ExternCrate(Some(crate_name)),
         ident: ast::Ident::from_str(name),
         id: ast::DUMMY_NODE_ID,
@@ -78,7 +78,7 @@ pub fn maybe_inject_crates_ref(mut krate: ast::Crate, alt_std_name: Option<Strin
             is_sugared_doc: false,
             span,
         }],
-        vis: ast::Visibility::Inherited,
+        vis: respan(span.empty(), ast::VisibilityKind::Inherited),
         node: ast::ItemKind::Use(P(ast::UseTree {
             prefix: ast::Path {
                 segments: ["{{root}}", name, "prelude", "v1"].into_iter().map(|name| {
diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs
index e73550d0719..7b119c576db 100644
--- a/src/libsyntax/test.rs
+++ b/src/libsyntax/test.rs
@@ -233,11 +233,11 @@ fn mk_reexport_mod(cx: &mut TestCtxt,
     let super_ = Ident::from_str("super");
 
     let items = tests.into_iter().map(|r| {
-        cx.ext_cx.item_use_simple(DUMMY_SP, ast::Visibility::Public,
+        cx.ext_cx.item_use_simple(DUMMY_SP, dummy_spanned(ast::VisibilityKind::Public),
                                   cx.ext_cx.path(DUMMY_SP, vec![super_, r]))
     }).chain(tested_submods.into_iter().map(|(r, sym)| {
         let path = cx.ext_cx.path(DUMMY_SP, vec![super_, r, sym]);
-        cx.ext_cx.item_use_simple_(DUMMY_SP, ast::Visibility::Public, r, path)
+        cx.ext_cx.item_use_simple_(DUMMY_SP, dummy_spanned(ast::VisibilityKind::Public), r, path)
     })).collect();
 
     let reexport_mod = ast::Mod {
@@ -253,7 +253,7 @@ fn mk_reexport_mod(cx: &mut TestCtxt,
         attrs: Vec::new(),
         id: ast::DUMMY_NODE_ID,
         node: ast::ItemKind::Mod(reexport_mod),
-        vis: ast::Visibility::Public,
+        vis: dummy_spanned(ast::VisibilityKind::Public),
         span: DUMMY_SP,
         tokens: None,
     })).pop().unwrap();
@@ -462,16 +462,16 @@ fn mk_std(cx: &TestCtxt) -> P<ast::Item> {
             prefix: path_node(vec![id_test]),
             kind: ast::UseTreeKind::Simple(id_test),
         })),
-         ast::Visibility::Public, keywords::Invalid.ident())
+         ast::VisibilityKind::Public, keywords::Invalid.ident())
     } else {
-        (ast::ItemKind::ExternCrate(None), ast::Visibility::Inherited, id_test)
+        (ast::ItemKind::ExternCrate(None), ast::VisibilityKind::Inherited, id_test)
     };
     P(ast::Item {
         id: ast::DUMMY_NODE_ID,
         ident,
         node: vi,
         attrs: vec![],
-        vis,
+        vis: dummy_spanned(vis),
         span: sp,
         tokens: None,
     })
@@ -513,7 +513,7 @@ fn mk_main(cx: &mut TestCtxt) -> P<ast::Item> {
         attrs: vec![main_attr],
         id: ast::DUMMY_NODE_ID,
         node: main,
-        vis: ast::Visibility::Public,
+        vis: dummy_spanned(ast::VisibilityKind::Public),
         span: sp,
         tokens: None,
     })
@@ -543,7 +543,7 @@ fn mk_test_module(cx: &mut TestCtxt) -> (P<ast::Item>, Option<P<ast::Item>>) {
         ident: mod_ident,
         attrs: vec![],
         node: item_,
-        vis: ast::Visibility::Public,
+        vis: dummy_spanned(ast::VisibilityKind::Public),
         span: DUMMY_SP,
         tokens: None,
     })).pop().unwrap();
@@ -562,7 +562,7 @@ fn mk_test_module(cx: &mut TestCtxt) -> (P<ast::Item>, Option<P<ast::Item>>) {
             ident: keywords::Invalid.ident(),
             attrs: vec![],
             node: ast::ItemKind::Use(P(use_path)),
-            vis: ast::Visibility::Inherited,
+            vis: dummy_spanned(ast::VisibilityKind::Inherited),
             span: DUMMY_SP,
             tokens: None,
         })).pop().unwrap()
diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
index bbb123dab28..4691ddafa36 100644
--- a/src/libsyntax/visit.rs
+++ b/src/libsyntax/visit.rs
@@ -811,7 +811,7 @@ pub fn walk_arm<'a, V: Visitor<'a>>(visitor: &mut V, arm: &'a Arm) {
 }
 
 pub fn walk_vis<'a, V: Visitor<'a>>(visitor: &mut V, vis: &'a Visibility) {
-    if let Visibility::Restricted { ref path, id } = *vis {
+    if let VisibilityKind::Restricted { ref path, id } = vis.node {
         visitor.visit_path(path, id);
     }
 }
diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs
index 0dfe9cb970e..1b3917efdd1 100644
--- a/src/libsyntax_ext/deriving/generic/mod.rs
+++ b/src/libsyntax_ext/deriving/generic/mod.rs
@@ -530,7 +530,7 @@ impl<'a> TraitDef<'a> {
                 id: ast::DUMMY_NODE_ID,
                 span: self.span,
                 ident,
-                vis: ast::Visibility::Inherited,
+                vis: respan(self.span.empty(), ast::VisibilityKind::Inherited),
                 defaultness: ast::Defaultness::Final,
                 attrs: Vec::new(),
                 generics: Generics::default(),
@@ -977,7 +977,7 @@ impl<'a> MethodDef<'a> {
             attrs: self.attributes.clone(),
             generics: fn_generics,
             span: trait_.span,
-            vis: ast::Visibility::Inherited,
+            vis: respan(trait_.span.empty(), ast::VisibilityKind::Inherited),
             defaultness: ast::Defaultness::Final,
             ident: method_ident,
             node: ast::ImplItemKind::Method(ast::MethodSig {
diff --git a/src/libsyntax_ext/global_asm.rs b/src/libsyntax_ext/global_asm.rs
index 81226ba599a..9605f6b5c5a 100644
--- a/src/libsyntax_ext/global_asm.rs
+++ b/src/libsyntax_ext/global_asm.rs
@@ -19,6 +19,7 @@
 /// therefore apply.
 
 use syntax::ast;
+use syntax::codemap::respan;
 use syntax::ext::base;
 use syntax::ext::base::*;
 use syntax::feature_gate;
@@ -59,7 +60,7 @@ pub fn expand_global_asm<'cx>(cx: &'cx mut ExtCtxt,
             asm,
             ctxt: cx.backtrace(),
         })),
-        vis: ast::Visibility::Inherited,
+        vis: respan(sp.empty(), ast::VisibilityKind::Inherited),
         span: sp,
         tokens: None,
     })))
diff --git a/src/libsyntax_ext/proc_macro_registrar.rs b/src/libsyntax_ext/proc_macro_registrar.rs
index 0ba21e6b366..e623779ce63 100644
--- a/src/libsyntax_ext/proc_macro_registrar.rs
+++ b/src/libsyntax_ext/proc_macro_registrar.rs
@@ -14,7 +14,7 @@ use errors;
 
 use syntax::ast::{self, Ident, NodeId};
 use syntax::attr;
-use syntax::codemap::{ExpnInfo, NameAndSpan, MacroAttribute};
+use syntax::codemap::{ExpnInfo, NameAndSpan, MacroAttribute, respan};
 use syntax::ext::base::ExtCtxt;
 use syntax::ext::build::AstBuilder;
 use syntax::ext::expand::ExpansionConfig;
@@ -103,7 +103,7 @@ impl<'a> CollectProcMacros<'a> {
     fn check_not_pub_in_root(&self, vis: &ast::Visibility, sp: Span) {
         if self.is_proc_macro_crate &&
            self.in_root &&
-           *vis == ast::Visibility::Public {
+           vis.node == ast::VisibilityKind::Public {
             self.handler.span_err(sp,
                                   "`proc-macro` crate types cannot \
                                    export any items other than functions \
@@ -181,7 +181,7 @@ impl<'a> CollectProcMacros<'a> {
             Vec::new()
         };
 
-        if self.in_root && item.vis == ast::Visibility::Public {
+        if self.in_root && item.vis.node == ast::VisibilityKind::Public {
             self.derives.push(ProcMacroDerive {
                 span: item.span,
                 trait_name,
@@ -206,7 +206,7 @@ impl<'a> CollectProcMacros<'a> {
             return;
         }
 
-        if self.in_root && item.vis == ast::Visibility::Public {
+        if self.in_root && item.vis.node == ast::VisibilityKind::Public {
             self.attr_macros.push(ProcMacroDef {
                 span: item.span,
                 function_name: item.ident,
@@ -229,7 +229,7 @@ impl<'a> CollectProcMacros<'a> {
             return;
         }
 
-        if self.in_root && item.vis == ast::Visibility::Public {
+        if self.in_root && item.vis.node == ast::VisibilityKind::Public {
             self.bang_macros.push(ProcMacroDef {
                 span: item.span,
                 function_name: item.ident,
@@ -439,12 +439,12 @@ fn mk_registrar(cx: &mut ExtCtxt,
     let derive_registrar = cx.attribute(span, derive_registrar);
     let func = func.map(|mut i| {
         i.attrs.push(derive_registrar);
-        i.vis = ast::Visibility::Public;
+        i.vis = respan(span, ast::VisibilityKind::Public);
         i
     });
     let ident = ast::Ident::with_empty_ctxt(Symbol::gensym("registrar"));
     let module = cx.item_mod(span, span, ident, Vec::new(), vec![krate, func]).map(|mut i| {
-        i.vis = ast::Visibility::Public;
+        i.vis = respan(span, ast::VisibilityKind::Public);
         i
     });
 
diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
index 294506625bc..0f6dbc39e21 100644
--- a/src/libsyntax_pos/lib.rs
+++ b/src/libsyntax_pos/lib.rs
@@ -216,6 +216,12 @@ impl Span {
         self.data().with_ctxt(ctxt)
     }
 
+    /// Returns a new span representing an empty span at the beginning of this span
+    #[inline]
+    pub fn empty(self) -> Span {
+        self.with_hi(self.lo())
+    }
+
     /// Returns `self` if `self` is not the dummy span, and `other` otherwise.
     pub fn substitute_dummy(self, other: Span) -> Span {
         if self.source_equal(&DUMMY_SP) { other } else { self }
diff --git a/src/test/ui/error-codes/E0449.stderr b/src/test/ui/error-codes/E0449.stderr
index 2270167303a..3587319ed0c 100644
--- a/src/test/ui/error-codes/E0449.stderr
+++ b/src/test/ui/error-codes/E0449.stderr
@@ -2,23 +2,21 @@ error[E0449]: unnecessary visibility qualifier
   --> $DIR/E0449.rs:17:1
    |
 17 | pub impl Bar {} //~ ERROR E0449
-   | ^^^^^^^^^^^^^^^ `pub` not needed here
+   | ^^^ `pub` not needed here
    |
    = note: place qualifiers on individual impl items instead
 
 error[E0449]: unnecessary visibility qualifier
   --> $DIR/E0449.rs:19:1
    |
-19 | / pub impl Foo for Bar { //~ ERROR E0449
-20 | |     pub fn foo() {} //~ ERROR E0449
-21 | | }
-   | |_^ `pub` not needed here
+19 | pub impl Foo for Bar { //~ ERROR E0449
+   | ^^^ `pub` not needed here
 
 error[E0449]: unnecessary visibility qualifier
   --> $DIR/E0449.rs:20:5
    |
 20 |     pub fn foo() {} //~ ERROR E0449
-   |     ^^^^^^^^^^^^^^^ `pub` not needed here
+   |     ^^^ `pub` not needed here
 
 error: aborting due to 3 previous errors