Skip to content

Commit 4593564

Browse files
borsbors
committed
Auto merge of #51866 - zackmdavis:hir_making_each_day_of_the_year, r=petrochenkov
add modifier keyword spans to hir::Visibility; improve unreachable-pub, private-no-mangle lint suggestions #50455 pointed out that the unreachable-pub suggestion for brace-grouped `use`s was bogus; #50476 partially ameliorated this by marking the suggestion as `Applicability::MaybeIncorrect`, but this is the actual fix. Meanwhile, another application of having spans available in `hir::Visibility` is found in the private-no-mangle lints, where we can now issue a suggestion to use `pub` if the item has a more restricted visibility marker (this seems much less likely to come up in practice than not having any visibility keyword at all, but thoroughness is a virtue). While we're there, we can also add a helpful note if the item does have a `pub` (but triggered the lint presumably because enclosing modules were private). ![hir_vis](https://user-images.githubusercontent.com/1076988/42018064-ca830290-7a65-11e8-9c4c-48bc846f861f.png) r? @nrc cc @Manishearth
2 parents 5df4311 + 43a0a65 commit 4593564

25 files changed

+345
-189
lines changed

‎src/librustc/hir/intravisit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1104,7 +1104,7 @@ pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
11041104
}
11051105

11061106
pub fn walk_vis<'v, V: Visitor<'v>>(visitor: &mut V, vis: &'v Visibility) {
1107-
if let Visibility::Restricted { ref path, id } = *vis {
1107+
if let VisibilityKind::Restricted { ref path, id } = vis.node {
11081108
visitor.visit_id(id);
11091109
visitor.visit_path(path, id)
11101110
}

‎src/librustc/hir/lowering.rs

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,7 @@ impl<'a> LoweringContext<'a> {
12961296
name: keywords::Invalid.name(),
12971297
attrs: Default::default(),
12981298
node: exist_ty_item_kind,
1299-
vis: hir::Visibility::Inherited,
1299+
vis: respan(span.shrink_to_lo(), hir::VisibilityKind::Inherited),
13001300
span: exist_ty_span,
13011301
};
13021302

@@ -2797,18 +2797,19 @@ impl<'a> LoweringContext<'a> {
27972797
let new_id = this.lower_node_id(new_node_id);
27982798
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
27992799
let item = hir::ItemUse(P(path), hir::UseKind::Single);
2800-
let vis = match vis {
2801-
hir::Visibility::Public => hir::Visibility::Public,
2802-
hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
2803-
hir::Visibility::Inherited => hir::Visibility::Inherited,
2804-
hir::Visibility::Restricted { ref path, id: _ } => {
2805-
hir::Visibility::Restricted {
2800+
let vis_kind = match vis.node {
2801+
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
2802+
hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
2803+
hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
2804+
hir::VisibilityKind::Restricted { ref path, id: _ } => {
2805+
hir::VisibilityKind::Restricted {
28062806
path: path.clone(),
28072807
// We are allocating a new NodeId here
28082808
id: this.next_id().node_id,
28092809
}
28102810
}
28112811
};
2812+
let vis = respan(vis.span, vis_kind);
28122813

28132814
this.items.insert(
28142815
new_id.node_id,
@@ -2869,18 +2870,19 @@ impl<'a> LoweringContext<'a> {
28692870
self.lower_use_tree(use_tree, &prefix, new_id, &mut vis, &mut name, &attrs);
28702871

28712872
self.with_hir_id_owner(new_id, |this| {
2872-
let vis = match vis {
2873-
hir::Visibility::Public => hir::Visibility::Public,
2874-
hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
2875-
hir::Visibility::Inherited => hir::Visibility::Inherited,
2876-
hir::Visibility::Restricted { ref path, id: _ } => {
2877-
hir::Visibility::Restricted {
2873+
let vis_kind = match vis.node {
2874+
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
2875+
hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
2876+
hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
2877+
hir::VisibilityKind::Restricted { ref path, id: _ } => {
2878+
hir::VisibilityKind::Restricted {
28782879
path: path.clone(),
28792880
// We are allocating a new NodeId here
28802881
id: this.next_id().node_id,
28812882
}
28822883
}
28832884
};
2885+
let vis = respan(vis.span, vis_kind);
28842886

28852887
this.items.insert(
28862888
new_id,
@@ -2901,7 +2903,7 @@ impl<'a> LoweringContext<'a> {
29012903
// the stability of `use a::{};`, to avoid it showing up as
29022904
// a re-export by accident when `pub`, e.g. in documentation.
29032905
let path = P(self.lower_path(id, &prefix, ParamMode::Explicit));
2904-
*vis = hir::Inherited;
2906+
*vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited);
29052907
hir::ItemUse(path, hir::UseKind::ListStem)
29062908
}
29072909
}
@@ -4284,19 +4286,20 @@ impl<'a> LoweringContext<'a> {
42844286
v: &Visibility,
42854287
explicit_owner: Option<NodeId>,
42864288
) -> hir::Visibility {
4287-
match v.node {
4288-
VisibilityKind::Public => hir::Public,
4289-
VisibilityKind::Crate(sugar) => hir::Visibility::Crate(sugar),
4290-
VisibilityKind::Restricted { ref path, id, .. } => hir::Visibility::Restricted {
4289+
let node = match v.node {
4290+
VisibilityKind::Public => hir::VisibilityKind::Public,
4291+
VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
4292+
VisibilityKind::Restricted { ref path, id } => hir::VisibilityKind::Restricted {
42914293
path: P(self.lower_path(id, path, ParamMode::Explicit)),
42924294
id: if let Some(owner) = explicit_owner {
42934295
self.lower_node_id_with_owner(id, owner).node_id
42944296
} else {
42954297
self.lower_node_id(id).node_id
42964298
},
42974299
},
4298-
VisibilityKind::Inherited => hir::Inherited,
4299-
}
4300+
VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
4301+
};
4302+
respan(v.span, node)
43004303
}
43014304

43024305
fn lower_defaultness(&mut self, d: Defaultness, has_value: bool) -> hir::Defaultness {

‎src/librustc/hir/map/collector.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,11 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
458458
}
459459

460460
fn visit_vis(&mut self, visibility: &'hir Visibility) {
461-
match *visibility {
462-
Visibility::Public |
463-
Visibility::Crate(_) |
464-
Visibility::Inherited => {}
465-
Visibility::Restricted { id, .. } => {
461+
match visibility.node {
462+
VisibilityKind::Public |
463+
VisibilityKind::Crate(_) |
464+
VisibilityKind::Inherited => {}
465+
VisibilityKind::Restricted { id, .. } => {
466466
self.insert(id, NodeVisibility(visibility));
467467
self.with_parent(id, |this| {
468468
intravisit::walk_vis(this, visibility);

‎src/librustc/hir/map/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,9 @@ impl<'hir> Map<'hir> {
10491049
Some(EntryStructCtor(_, _, _)) => self.expect_item(self.get_parent(id)).span,
10501050
Some(EntryLifetime(_, _, lifetime)) => lifetime.span,
10511051
Some(EntryGenericParam(_, _, param)) => param.span,
1052-
Some(EntryVisibility(_, _, &Visibility::Restricted { ref path, .. })) => path.span,
1052+
Some(EntryVisibility(_, _, &Spanned {
1053+
node: VisibilityKind::Restricted { ref path, .. }, ..
1054+
})) => path.span,
10531055
Some(EntryVisibility(_, _, v)) => bug!("unexpected Visibility {:?}", v),
10541056
Some(EntryLocal(_, _, local)) => local.span,
10551057
Some(EntryMacroDef(_, macro_def)) => macro_def.span,

‎src/librustc/hir/mod.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ pub use self::Stmt_::*;
2424
pub use self::Ty_::*;
2525
pub use self::UnOp::*;
2626
pub use self::UnsafeSource::*;
27-
pub use self::Visibility::{Public, Inherited};
2827

2928
use hir::def::Def;
3029
use hir::def_id::{DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX};
@@ -1929,22 +1928,30 @@ pub struct PolyTraitRef {
19291928
pub span: Span,
19301929
}
19311930

1931+
pub type Visibility = Spanned<VisibilityKind>;
1932+
19321933
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
1933-
pub enum Visibility {
1934+
pub enum VisibilityKind {
19341935
Public,
19351936
Crate(CrateSugar),
19361937
Restricted { path: P<Path>, id: NodeId },
19371938
Inherited,
19381939
}
19391940

1940-
impl Visibility {
1941+
impl VisibilityKind {
1942+
pub fn is_pub(&self) -> bool {
1943+
match *self {
1944+
VisibilityKind::Public => true,
1945+
_ => false
1946+
}
1947+
}
1948+
19411949
pub fn is_pub_restricted(&self) -> bool {
1942-
use self::Visibility::*;
1943-
match self {
1944-
&Public |
1945-
&Inherited => false,
1946-
&Crate(_) |
1947-
&Restricted { .. } => true,
1950+
match *self {
1951+
VisibilityKind::Public |
1952+
VisibilityKind::Inherited => false,
1953+
VisibilityKind::Crate(..) |
1954+
VisibilityKind::Restricted { .. } => true,
19481955
}
19491956
}
19501957
}

‎src/librustc/hir/print.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub use self::AnnNode::*;
1212

1313
use rustc_target::spec::abi::Abi;
1414
use syntax::ast;
15-
use syntax::codemap::CodeMap;
15+
use syntax::codemap::{CodeMap, Spanned};
1616
use syntax::parse::ParseSess;
1717
use syntax::parse::lexer::comments;
1818
use syntax::print::pp::{self, Breaks};
@@ -839,11 +839,11 @@ impl<'a> State<'a> {
839839
}
840840

841841
pub fn print_visibility(&mut self, vis: &hir::Visibility) -> io::Result<()> {
842-
match *vis {
843-
hir::Public => self.word_nbsp("pub")?,
844-
hir::Visibility::Crate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
845-
hir::Visibility::Crate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
846-
hir::Visibility::Restricted { ref path, .. } => {
842+
match vis.node {
843+
hir::VisibilityKind::Public => self.word_nbsp("pub")?,
844+
hir::VisibilityKind::Crate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
845+
hir::VisibilityKind::Crate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
846+
hir::VisibilityKind::Restricted { ref path, .. } => {
847847
self.s.word("pub(")?;
848848
if path.segments.len() == 1 &&
849849
path.segments[0].ident.name == keywords::Super.name() {
@@ -856,7 +856,7 @@ impl<'a> State<'a> {
856856
}
857857
self.word_nbsp(")")?;
858858
}
859-
hir::Inherited => ()
859+
hir::VisibilityKind::Inherited => ()
860860
}
861861

862862
Ok(())
@@ -952,17 +952,21 @@ impl<'a> State<'a> {
952952
self.print_outer_attributes(&ti.attrs)?;
953953
match ti.node {
954954
hir::TraitItemKind::Const(ref ty, default) => {
955-
self.print_associated_const(ti.ident, &ty, default, &hir::Inherited)?;
955+
let vis = Spanned { span: syntax_pos::DUMMY_SP,
956+
node: hir::VisibilityKind::Inherited };
957+
self.print_associated_const(ti.ident, &ty, default, &vis)?;
956958
}
957959
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(ref arg_names)) => {
958-
self.print_method_sig(ti.ident, sig, &ti.generics, &hir::Inherited, arg_names,
959-
None)?;
960+
let vis = Spanned { span: syntax_pos::DUMMY_SP,
961+
node: hir::VisibilityKind::Inherited };
962+
self.print_method_sig(ti.ident, sig, &ti.generics, &vis, arg_names, None)?;
960963
self.s.word(";")?;
961964
}
962965
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => {
966+
let vis = Spanned { span: syntax_pos::DUMMY_SP,
967+
node: hir::VisibilityKind::Inherited };
963968
self.head("")?;
964-
self.print_method_sig(ti.ident, sig, &ti.generics, &hir::Inherited, &[],
965-
Some(body))?;
969+
self.print_method_sig(ti.ident, sig, &ti.generics, &vis, &[], Some(body))?;
966970
self.nbsp()?;
967971
self.end()?; // need to close a box
968972
self.end()?; // need to close a box
@@ -2266,7 +2270,8 @@ impl<'a> State<'a> {
22662270
},
22672271
name,
22682272
&generics,
2269-
&hir::Inherited,
2273+
&Spanned { span: syntax_pos::DUMMY_SP,
2274+
node: hir::VisibilityKind::Inherited },
22702275
arg_names,
22712276
None)?;
22722277
self.end()

‎src/librustc/ich/impls_hir.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -710,20 +710,20 @@ impl_stable_hash_for!(enum ::syntax::ast::CrateSugar {
710710
PubCrate,
711711
});
712712

713-
impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility {
713+
impl<'a> HashStable<StableHashingContext<'a>> for hir::VisibilityKind {
714714
fn hash_stable<W: StableHasherResult>(&self,
715715
hcx: &mut StableHashingContext<'a>,
716716
hasher: &mut StableHasher<W>) {
717717
mem::discriminant(self).hash_stable(hcx, hasher);
718718
match *self {
719-
hir::Visibility::Public |
720-
hir::Visibility::Inherited => {
719+
hir::VisibilityKind::Public |
720+
hir::VisibilityKind::Inherited => {
721721
// No fields to hash.
722722
}
723-
hir::Visibility::Crate(sugar) => {
723+
hir::VisibilityKind::Crate(sugar) => {
724724
sugar.hash_stable(hcx, hasher);
725725
}
726-
hir::Visibility::Restricted { ref path, id } => {
726+
hir::VisibilityKind::Restricted { ref path, id } => {
727727
hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
728728
id.hash_stable(hcx, hasher);
729729
});
@@ -733,6 +733,8 @@ impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility {
733733
}
734734
}
735735

736+
impl_stable_hash_for_spanned!(hir::VisibilityKind);
737+
736738
impl<'a> HashStable<StableHashingContext<'a>> for hir::Defaultness {
737739
fn hash_stable<W: StableHasherResult>(&self,
738740
hcx: &mut StableHashingContext<'a>,

‎src/librustc/middle/dead.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
161161
intravisit::walk_item(self, &item);
162162
}
163163
hir::ItemEnum(..) => {
164-
self.inherited_pub_visibility = item.vis == hir::Public;
164+
self.inherited_pub_visibility = item.vis.node.is_pub();
165165
intravisit::walk_item(self, &item);
166166
}
167167
hir::ItemFn(..)
@@ -216,7 +216,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> {
216216
let has_repr_c = self.repr_has_repr_c;
217217
let inherited_pub_visibility = self.inherited_pub_visibility;
218218
let live_fields = def.fields().iter().filter(|f| {
219-
has_repr_c || inherited_pub_visibility || f.vis == hir::Public
219+
has_repr_c || inherited_pub_visibility || f.vis.node.is_pub()
220220
});
221221
self.live_symbols.extend(live_fields.map(|f| f.id));
222222

‎src/librustc/ty/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,16 +268,16 @@ impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> {
268268

269269
impl Visibility {
270270
pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self {
271-
match *visibility {
272-
hir::Public => Visibility::Public,
273-
hir::Visibility::Crate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
274-
hir::Visibility::Restricted { ref path, .. } => match path.def {
271+
match visibility.node {
272+
hir::VisibilityKind::Public => Visibility::Public,
273+
hir::VisibilityKind::Crate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
274+
hir::VisibilityKind::Restricted { ref path, .. } => match path.def {
275275
// If there is no resolution, `resolve` will have already reported an error, so
276276
// assume that the visibility is public to avoid reporting more privacy errors.
277277
Def::Err => Visibility::Public,
278278
def => Visibility::Restricted(def.def_id()),
279279
},
280-
hir::Inherited => {
280+
hir::VisibilityKind::Inherited => {
281281
Visibility::Restricted(tcx.hir.get_module_parent(id))
282282
}
283283
}

0 commit comments

Comments
 (0)