Skip to content

Commit 063deba

Browse files
borsbors
committed
Auto merge of rust-lang#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 (rust-lang/rustfmt#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.
2 parents 9284353 + 8e9fa57 commit 063deba

File tree

26 files changed

+174
-129
lines changed

26 files changed

+174
-129
lines changed

‎src/librustc/hir/lowering.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3362,10 +3362,10 @@ impl<'a> LoweringContext<'a> {
33623362
v: &Visibility,
33633363
explicit_owner: Option<NodeId>)
33643364
-> hir::Visibility {
3365-
match *v {
3366-
Visibility::Public => hir::Public,
3367-
Visibility::Crate(..) => hir::Visibility::Crate,
3368-
Visibility::Restricted { ref path, id } => {
3365+
match v.node {
3366+
VisibilityKind::Public => hir::Public,
3367+
VisibilityKind::Crate(..) => hir::Visibility::Crate,
3368+
VisibilityKind::Restricted { ref path, id, .. } => {
33693369
hir::Visibility::Restricted {
33703370
path: P(self.lower_path(id, path, ParamMode::Explicit, true)),
33713371
id: if let Some(owner) = explicit_owner {
@@ -3375,7 +3375,7 @@ impl<'a> LoweringContext<'a> {
33753375
}
33763376
}
33773377
}
3378-
Visibility::Inherited => hir::Inherited,
3378+
VisibilityKind::Inherited => hir::Inherited,
33793379
}
33803380
}
33813381

‎src/librustc_allocator/expand.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ use rustc_errors;
1313
use syntax::abi::Abi;
1414
use syntax::ast::{Crate, Attribute, LitKind, StrStyle, ExprKind};
1515
use syntax::ast::{Unsafety, Constness, Generics, Mutability, Ty, Mac, Arg};
16-
use syntax::ast::{self, Ident, Item, ItemKind, TyKind, Visibility, Expr};
16+
use syntax::ast::{self, Ident, Item, ItemKind, TyKind, VisibilityKind, Expr};
1717
use syntax::attr;
18-
use syntax::codemap::dummy_spanned;
18+
use syntax::codemap::{dummy_spanned, respan};
1919
use syntax::codemap::{ExpnInfo, NameAndSpan, MacroAttribute};
2020
use syntax::ext::base::ExtCtxt;
2121
use syntax::ext::base::Resolver;
@@ -97,7 +97,11 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> {
9797
]);
9898
let mut items = vec![
9999
f.cx.item_extern_crate(f.span, f.alloc),
100-
f.cx.item_use_simple(f.span, Visibility::Inherited, super_path),
100+
f.cx.item_use_simple(
101+
f.span,
102+
respan(f.span.empty(), VisibilityKind::Inherited),
103+
super_path,
104+
),
101105
];
102106
for method in ALLOCATOR_METHODS {
103107
items.push(f.allocator_fn(method));

‎src/librustc_metadata/cstore_impl.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use std::rc::Rc;
3434

3535
use syntax::ast;
3636
use syntax::attr;
37+
use syntax::codemap;
3738
use syntax::ext::base::SyntaxExtension;
3839
use syntax::parse::filemap_to_stream;
3940
use syntax::symbol::Symbol;
@@ -496,7 +497,7 @@ impl CrateStore for cstore::CStore {
496497
tokens: body.into(),
497498
legacy: def.legacy,
498499
}),
499-
vis: ast::Visibility::Inherited,
500+
vis: codemap::respan(local_span.empty(), ast::VisibilityKind::Inherited),
500501
tokens: None,
501502
})
502503
}

‎src/librustc_passes/ast_validation.rs

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ impl<'a> AstValidator<'a> {
5858
}
5959
}
6060

61-
fn invalid_visibility(&self, vis: &Visibility, span: Span, note: Option<&str>) {
62-
if vis != &Visibility::Inherited {
61+
fn invalid_visibility(&self, vis: &Visibility, note: Option<&str>) {
62+
if vis.node != VisibilityKind::Inherited {
6363
let mut err = struct_span_err!(self.session,
64-
span,
64+
vis.span,
6565
E0449,
6666
"unnecessary visibility qualifier");
67-
if vis == &Visibility::Public {
68-
err.span_label(span, "`pub` not needed here");
67+
if vis.node == VisibilityKind::Public {
68+
err.span_label(vis.span, "`pub` not needed here");
6969
}
7070
if let Some(note) = note {
7171
err.note(note);
@@ -216,7 +216,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
216216
fn visit_item(&mut self, item: &'a Item) {
217217
match item.node {
218218
ItemKind::Impl(unsafety, polarity, _, _, Some(..), ref ty, ref impl_items) => {
219-
self.invalid_visibility(&item.vis, item.span, None);
219+
self.invalid_visibility(&item.vis, None);
220220
if ty.node == TyKind::Err {
221221
self.err_handler()
222222
.struct_span_err(item.span, "`impl Trait for .. {}` is an obsolete syntax")
@@ -226,15 +226,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
226226
span_err!(self.session, item.span, E0198, "negative impls cannot be unsafe");
227227
}
228228
for impl_item in impl_items {
229-
self.invalid_visibility(&impl_item.vis, impl_item.span, None);
229+
self.invalid_visibility(&impl_item.vis, None);
230230
if let ImplItemKind::Method(ref sig, _) = impl_item.node {
231231
self.check_trait_fn_not_const(sig.constness);
232232
}
233233
}
234234
}
235235
ItemKind::Impl(unsafety, polarity, defaultness, _, None, _, _) => {
236236
self.invalid_visibility(&item.vis,
237-
item.span,
238237
Some("place qualifiers on individual impl items instead"));
239238
if unsafety == Unsafety::Unsafe {
240239
span_err!(self.session, item.span, E0197, "inherent impls cannot be unsafe");
@@ -247,16 +246,16 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
247246
}
248247
}
249248
ItemKind::ForeignMod(..) => {
250-
self.invalid_visibility(&item.vis,
251-
item.span,
252-
Some("place qualifiers on individual foreign items \
253-
instead"));
249+
self.invalid_visibility(
250+
&item.vis,
251+
Some("place qualifiers on individual foreign items instead"),
252+
);
254253
}
255254
ItemKind::Enum(ref def, _) => {
256255
for variant in &def.variants {
257256
self.invalid_non_exhaustive_attribute(variant);
258257
for field in variant.node.data.fields() {
259-
self.invalid_visibility(&field.vis, field.span, None);
258+
self.invalid_visibility(&field.vis, None);
260259
}
261260
}
262261
}
@@ -359,8 +358,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
359358
}
360359

361360
fn visit_vis(&mut self, vis: &'a Visibility) {
362-
match *vis {
363-
Visibility::Restricted { ref path, .. } => {
361+
match vis.node {
362+
VisibilityKind::Restricted { ref path, .. } => {
364363
path.segments.iter().find(|segment| segment.parameters.is_some()).map(|segment| {
365364
self.err_handler().span_err(segment.parameters.as_ref().unwrap().span(),
366365
"generic arguments in visibility path");

‎src/librustc_resolve/check_unused.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl<'a, 'b> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b> {
8686
// because this means that they were generated in some fashion by the
8787
// compiler and we don't need to consider them.
8888
if let ast::ItemKind::Use(..) = item.node {
89-
if item.vis == ast::Visibility::Public || item.span.source_equal(&DUMMY_SP) {
89+
if item.vis.node == ast::VisibilityKind::Public || item.span.source_equal(&DUMMY_SP) {
9090
return;
9191
}
9292
}

‎src/librustc_resolve/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3796,13 +3796,15 @@ impl<'a> Resolver<'a> {
37963796
}
37973797

37983798
fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility {
3799-
match *vis {
3800-
ast::Visibility::Public => ty::Visibility::Public,
3801-
ast::Visibility::Crate(..) => ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
3802-
ast::Visibility::Inherited => {
3799+
match vis.node {
3800+
ast::VisibilityKind::Public => ty::Visibility::Public,
3801+
ast::VisibilityKind::Crate(..) => {
3802+
ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))
3803+
}
3804+
ast::VisibilityKind::Inherited => {
38033805
ty::Visibility::Restricted(self.current_module.normal_ancestor_id)
38043806
}
3805-
ast::Visibility::Restricted { ref path, id } => {
3807+
ast::VisibilityKind::Restricted { ref path, id, .. } => {
38063808
let def = self.smart_resolve_path(id, None, path,
38073809
PathSource::Visibility).base_def();
38083810
if def == Def::Err {

‎src/librustc_save_analysis/dump_visitor.rs

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use syntax::print::pprust::{
4343
ty_to_string
4444
};
4545
use syntax::ptr::P;
46-
use syntax::codemap::{Spanned, DUMMY_SP};
46+
use syntax::codemap::{Spanned, DUMMY_SP, respan};
4747
use syntax_pos::*;
4848

4949
use {escape, generated_code, lower_attributes, PathCollector, SaveContext};
@@ -65,12 +65,19 @@ macro_rules! down_cast_data {
6565
}
6666

6767
macro_rules! access_from {
68+
($save_ctxt:expr, $vis:expr, $id:expr) => {
69+
Access {
70+
public: $vis.node == ast::VisibilityKind::Public,
71+
reachable: $save_ctxt.analysis.access_levels.is_reachable($id),
72+
}
73+
};
74+
6875
($save_ctxt:expr, $item:expr) => {
6976
Access {
70-
public: $item.vis == ast::Visibility::Public,
77+
public: $item.vis.node == ast::VisibilityKind::Public,
7178
reachable: $save_ctxt.analysis.access_levels.is_reachable($item.id),
7279
}
73-
}
80+
};
7481
}
7582

7683
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> {
405412

406413
method_data.value = sig_str;
407414
method_data.sig = sig::method_signature(id, name, generics, sig, &self.save_ctxt);
408-
self.dumper.dump_def(
409-
&Access {
410-
public: vis == ast::Visibility::Public,
411-
reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
412-
},
413-
method_data);
415+
self.dumper.dump_def(&access_from!(self.save_ctxt, vis, id), method_data);
414416
}
415417

416418
// walk arg and return types
@@ -543,10 +545,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
543545
let span = self.span_from_span(sub_span.expect("No span found for variable"));
544546

545547
self.dumper.dump_def(
546-
&Access {
547-
public: vis == ast::Visibility::Public,
548-
reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
549-
},
548+
&access_from!(self.save_ctxt, vis, id),
550549
Def {
551550
kind: DefKind::Const,
552551
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> {
597596
.iter()
598597
.enumerate()
599598
.filter_map(|(i, f)| {
600-
if include_priv_fields || f.vis == ast::Visibility::Public {
599+
if include_priv_fields || f.vis.node == ast::VisibilityKind::Public {
601600
f.ident
602601
.map(|i| i.to_string())
603602
.or_else(|| Some(i.to_string()))
@@ -1135,6 +1134,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
11351134

11361135
fn process_trait_item(&mut self, trait_item: &'l ast::TraitItem, trait_id: DefId) {
11371136
self.process_macro_use(trait_item.span);
1137+
let vis_span = trait_item.span.empty();
11381138
match trait_item.node {
11391139
ast::TraitItemKind::Const(ref ty, ref expr) => {
11401140
self.process_assoc_const(
@@ -1144,7 +1144,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
11441144
&ty,
11451145
expr.as_ref().map(|e| &**e),
11461146
trait_id,
1147-
ast::Visibility::Public,
1147+
respan(vis_span, ast::VisibilityKind::Public),
11481148
&trait_item.attrs,
11491149
);
11501150
}
@@ -1155,7 +1155,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
11551155
trait_item.id,
11561156
trait_item.ident,
11571157
&trait_item.generics,
1158-
ast::Visibility::Public,
1158+
respan(vis_span, ast::VisibilityKind::Public),
11591159
trait_item.span,
11601160
);
11611161
}
@@ -1259,10 +1259,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
12591259

12601260
// The access is calculated using the current tree ID, but with the root tree's visibility
12611261
// (since nested trees don't have their own visibility).
1262-
let access = Access {
1263-
public: root_item.vis == ast::Visibility::Public,
1264-
reachable: self.save_ctxt.analysis.access_levels.is_reachable(id),
1265-
};
1262+
let access = access_from!(self.save_ctxt, root_item.vis, id);
12661263

12671264
// The parent def id of a given use tree is always the enclosing item.
12681265
let parent = self.save_ctxt.tcx.hir.opt_local_def_id(id)

‎src/libsyntax/ast.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,10 +1937,12 @@ pub enum CrateSugar {
19371937
JustCrate,
19381938
}
19391939

1940+
pub type Visibility = Spanned<VisibilityKind>;
1941+
19401942
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
1941-
pub enum Visibility {
1943+
pub enum VisibilityKind {
19421944
Public,
1943-
Crate(Span, CrateSugar),
1945+
Crate(CrateSugar),
19441946
Restricted { path: P<Path>, id: NodeId },
19451947
Inherited,
19461948
}

‎src/libsyntax/diagnostics/plugin.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::env;
1414

1515
use ast;
1616
use ast::{Ident, Name};
17+
use codemap;
1718
use syntax_pos::Span;
1819
use ext::base::{ExtCtxt, MacEager, MacResult};
1920
use ext::build::AstBuilder;
@@ -234,7 +235,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,
234235
ty,
235236
expr,
236237
),
237-
vis: ast::Visibility::Public,
238+
vis: codemap::respan(span.empty(), ast::VisibilityKind::Public),
238239
span,
239240
tokens: None,
240241
})

‎src/libsyntax/ext/build.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
987987
attrs,
988988
id: ast::DUMMY_NODE_ID,
989989
node,
990-
vis: ast::Visibility::Inherited,
990+
vis: respan(span.empty(), ast::VisibilityKind::Inherited),
991991
span,
992992
tokens: None,
993993
})
@@ -1033,7 +1033,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
10331033
span: ty.span,
10341034
ty,
10351035
ident: None,
1036-
vis: ast::Visibility::Inherited,
1036+
vis: respan(span.empty(), ast::VisibilityKind::Inherited),
10371037
attrs: Vec::new(),
10381038
id: ast::DUMMY_NODE_ID,
10391039
}

‎src/libsyntax/ext/expand.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use ast::{self, Block, Ident, NodeId, PatKind, Path};
1212
use ast::{MacStmtStyle, StmtKind, ItemKind};
1313
use attr::{self, HasAttrs};
14-
use codemap::{ExpnInfo, NameAndSpan, MacroBang, MacroAttribute, dummy_spanned};
14+
use codemap::{ExpnInfo, NameAndSpan, MacroBang, MacroAttribute, dummy_spanned, respan};
1515
use config::{is_test_or_bench, StripUnconfigured};
1616
use errors::FatalError;
1717
use ext::base::*;
@@ -238,7 +238,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
238238
node: ast::ItemKind::Mod(krate.module),
239239
ident: keywords::Invalid.ident(),
240240
id: ast::DUMMY_NODE_ID,
241-
vis: ast::Visibility::Public,
241+
vis: respan(krate.span.empty(), ast::VisibilityKind::Public),
242242
tokens: None,
243243
})));
244244

@@ -1022,7 +1022,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
10221022
// Ensure that test functions are accessible from the test harness.
10231023
ast::ItemKind::Fn(..) if self.cx.ecfg.should_test => {
10241024
if item.attrs.iter().any(|attr| is_test_or_bench(attr)) {
1025-
item = item.map(|mut item| { item.vis = ast::Visibility::Public; item });
1025+
item = item.map(|mut item| {
1026+
item.vis = respan(item.vis.span, ast::VisibilityKind::Public);
1027+
item
1028+
});
10261029
}
10271030
noop_fold_item(item, self)
10281031
}

‎src/libsyntax/ext/placeholders.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub fn placeholder(kind: ExpansionKind, id: ast::NodeId) -> Expansion {
3333
let ident = keywords::Invalid.ident();
3434
let attrs = Vec::new();
3535
let generics = ast::Generics::default();
36-
let vis = ast::Visibility::Inherited;
36+
let vis = dummy_spanned(ast::VisibilityKind::Inherited);
3737
let span = DUMMY_SP;
3838
let expr_placeholder = || P(ast::Expr {
3939
id, span,

‎src/libsyntax/ext/quote.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
// except according to those terms.
1010

1111
use ast::{self, Arg, Arm, Block, Expr, Item, Pat, Stmt, Ty};
12+
use codemap::respan;
1213
use syntax_pos::Span;
1314
use ext::base::ExtCtxt;
1415
use ext::base;
@@ -855,7 +856,12 @@ fn expand_wrapper(cx: &ExtCtxt,
855856
let mut stmts = imports.iter().map(|path| {
856857
// make item: `use ...;`
857858
let path = path.iter().map(|s| s.to_string()).collect();
858-
cx.stmt_item(sp, cx.item_use_glob(sp, ast::Visibility::Inherited, ids_ext(path)))
859+
let use_item = cx.item_use_glob(
860+
sp,
861+
respan(sp.empty(), ast::VisibilityKind::Inherited),
862+
ids_ext(path),
863+
);
864+
cx.stmt_item(sp, use_item)
859865
}).chain(Some(stmt_let_ext_cx)).collect::<Vec<_>>();
860866
stmts.push(cx.stmt_expr(expr));
861867

0 commit comments

Comments
 (0)