From 9df9c9df7bdcd688bcad9e91b9e2669f01d9f858 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Sat, 30 Jun 2018 20:22:19 -0700
Subject: [PATCH 1/7] choose a less arbitrary span when parsing the empty
visibility modifier
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Visibility spans were added to the AST in #47799 (d6bdf296) as a
`Spanned<_>`—which means that we need to choose a span even in the case
of inherited visibility (what you get when there's no `pub` &c. keyword
at all). That initial implementation's choice is pretty
counterintuitive, which could matter if we want to use it as a site to
suggest inserting a visibility modifier, &c.
(The phrase "Schelling span" in the comment is meant in analogy to the
game-theoretic concept of a "Schelling point", a value that is chosen
simply because it's what one can expect to agree upon with other agents
in the absence of explicit coördination.)
---
src/libsyntax/parse/parser.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 673157d0ffa..1f062656b81 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -6032,7 +6032,10 @@ impl<'a> Parser<'a> {
}
if !self.eat_keyword(keywords::Pub) {
- return Ok(respan(self.prev_span, VisibilityKind::Inherited))
+ // We need a span for our `Spanned<VisibilityKind>`, but there's inherently no
+ // keyword to grab a span from for inherited visibility; an empty span at the
+ // beginning of the current token would seem to be the "Schelling span".
+ return Ok(respan(self.span.shrink_to_lo(), VisibilityKind::Inherited))
}
let lo = self.prev_span;
From 4ae89129e1beefbe80cca4a13f6fd6e783653926 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Sat, 30 Jun 2018 20:34:18 -0700
Subject: [PATCH 2/7] in which hir::Visibility recalls whence it came (i.e.,
becomes Spanned)
There are at least a couple (and plausibly even three) diagnostics that
could use the spans of visibility modifiers in order to be reliably
correct (rather than hacking and munging surrounding spans to try to
infer where the visibility keyword must have been).
We follow the naming convention established by the other `Spanned` HIR
nodes: the "outer" type alias gets the "prime" node-type name, the
"inner" enum gets the name suffixed with an underscore, and the variant
names are prefixed with the prime name and `pub use` exported from here
(from HIR).
Thanks to veteran reviewer Vadim Petrochenkov for suggesting this
uniform approach. (A previous draft, based on the reasoning that
`Visibility::Inherited` should not have a span, tried to hack in a named
`span` field on `Visibility::Restricted` and a positional field on
`Public` and `Crate`. This was ... not so uniform.)
---
src/librustc/hir/intravisit.rs | 2 +-
src/librustc/hir/lowering.rs | 43 +++++++++++++++--------------
src/librustc/hir/map/collector.rs | 10 +++----
src/librustc/hir/map/mod.rs | 4 ++-
src/librustc/hir/mod.rs | 34 ++++++++++++++---------
src/librustc/hir/print.rs | 27 +++++++++---------
src/librustc/ich/impls_hir.rs | 12 ++++----
src/librustc/middle/dead.rs | 4 +--
src/librustc/ty/mod.rs | 10 +++----
src/librustc_lint/builtin.rs | 10 +++----
src/librustc_metadata/encoder.rs | 4 ++-
src/librustc_privacy/lib.rs | 18 ++++++------
src/librustc_save_analysis/lib.rs | 4 ++-
src/librustc_typeck/check_unused.rs | 3 +-
src/librustdoc/clean/mod.rs | 16 +++++------
src/librustdoc/doctree.rs | 3 +-
src/librustdoc/visit_ast.rs | 16 ++++++-----
17 files changed, 121 insertions(+), 99 deletions(-)
diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs
index c5e5fa65fc6..7173f670cd5 100644
--- a/src/librustc/hir/intravisit.rs
+++ b/src/librustc/hir/intravisit.rs
@@ -1104,7 +1104,7 @@ pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
}
pub fn walk_vis<'v, V: Visitor<'v>>(visitor: &mut V, vis: &'v Visibility) {
- if let Visibility::Restricted { ref path, id } = *vis {
+ if let VisibilityRestricted { ref path, id } = vis.node {
visitor.visit_id(id);
visitor.visit_path(path, id)
}
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 0bd5b6b627f..4db12c95168 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -1285,7 +1285,7 @@ impl<'a> LoweringContext<'a> {
name: keywords::Invalid.name(),
attrs: Default::default(),
node: exist_ty_item_kind,
- vis: hir::Visibility::Inherited,
+ vis: respan(span.shrink_to_lo(), hir::VisibilityInherited),
span: exist_ty_span,
};
@@ -2770,18 +2770,19 @@ impl<'a> LoweringContext<'a> {
let new_id = this.lower_node_id(new_node_id);
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
let item = hir::ItemUse(P(path), hir::UseKind::Single);
- let vis = match vis {
- hir::Visibility::Public => hir::Visibility::Public,
- hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
- hir::Visibility::Inherited => hir::Visibility::Inherited,
- hir::Visibility::Restricted { ref path, id: _ } => {
- hir::Visibility::Restricted {
+ let vis_kind = match vis.node {
+ hir::VisibilityPublic => hir::VisibilityPublic,
+ hir::VisibilityCrate(sugar) => hir::VisibilityCrate(sugar),
+ hir::VisibilityInherited => hir::VisibilityInherited,
+ hir::VisibilityRestricted { ref path, id: _ } => {
+ hir::VisibilityRestricted {
path: path.clone(),
// We are allocating a new NodeId here
id: this.next_id().node_id,
}
}
};
+ let vis = respan(vis.span, vis_kind);
this.items.insert(
new_id.node_id,
@@ -2842,18 +2843,19 @@ impl<'a> LoweringContext<'a> {
self.lower_use_tree(use_tree, &prefix, new_id, &mut vis, &mut name, &attrs);
self.with_hir_id_owner(new_id, |this| {
- let vis = match vis {
- hir::Visibility::Public => hir::Visibility::Public,
- hir::Visibility::Crate(sugar) => hir::Visibility::Crate(sugar),
- hir::Visibility::Inherited => hir::Visibility::Inherited,
- hir::Visibility::Restricted { ref path, id: _ } => {
- hir::Visibility::Restricted {
+ let vis_kind = match vis.node {
+ hir::VisibilityPublic => hir::VisibilityPublic,
+ hir::VisibilityCrate(sugar) => hir::VisibilityCrate(sugar),
+ hir::VisibilityInherited => hir::VisibilityInherited,
+ hir::VisibilityRestricted { ref path, id: _ } => {
+ hir::VisibilityRestricted {
path: path.clone(),
// We are allocating a new NodeId here
id: this.next_id().node_id,
}
}
};
+ let vis = respan(vis.span, vis_kind);
this.items.insert(
new_id,
@@ -2874,7 +2876,7 @@ impl<'a> LoweringContext<'a> {
// the stability of `use a::{};`, to avoid it showing up as
// a re-export by accident when `pub`, e.g. in documentation.
let path = P(self.lower_path(id, &prefix, ParamMode::Explicit));
- *vis = hir::Inherited;
+ *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityInherited);
hir::ItemUse(path, hir::UseKind::ListStem)
}
}
@@ -4274,10 +4276,10 @@ impl<'a> LoweringContext<'a> {
v: &Visibility,
explicit_owner: Option<NodeId>,
) -> hir::Visibility {
- match v.node {
- VisibilityKind::Public => hir::Public,
- VisibilityKind::Crate(sugar) => hir::Visibility::Crate(sugar),
- VisibilityKind::Restricted { ref path, id, .. } => hir::Visibility::Restricted {
+ let node = match v.node {
+ VisibilityKind::Public => hir::VisibilityPublic,
+ VisibilityKind::Crate(sugar) => hir::VisibilityCrate(sugar),
+ VisibilityKind::Restricted { ref path, id } => hir::VisibilityRestricted {
path: P(self.lower_path(id, path, ParamMode::Explicit)),
id: if let Some(owner) = explicit_owner {
self.lower_node_id_with_owner(id, owner).node_id
@@ -4285,8 +4287,9 @@ impl<'a> LoweringContext<'a> {
self.lower_node_id(id).node_id
},
},
- VisibilityKind::Inherited => hir::Inherited,
- }
+ VisibilityKind::Inherited => hir::VisibilityInherited,
+ };
+ respan(v.span, node)
}
fn lower_defaultness(&mut self, d: Defaultness, has_value: bool) -> hir::Defaultness {
diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs
index f0fc0d9b1c2..55ad73515c0 100644
--- a/src/librustc/hir/map/collector.rs
+++ b/src/librustc/hir/map/collector.rs
@@ -458,11 +458,11 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
}
fn visit_vis(&mut self, visibility: &'hir Visibility) {
- match *visibility {
- Visibility::Public |
- Visibility::Crate(_) |
- Visibility::Inherited => {}
- Visibility::Restricted { id, .. } => {
+ match visibility.node {
+ VisibilityPublic |
+ VisibilityCrate(_) |
+ VisibilityInherited => {}
+ VisibilityRestricted { id, .. } => {
self.insert(id, NodeVisibility(visibility));
self.with_parent(id, |this| {
intravisit::walk_vis(this, visibility);
diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
index dbf99cf30e5..a5c0a5e33f7 100644
--- a/src/librustc/hir/map/mod.rs
+++ b/src/librustc/hir/map/mod.rs
@@ -1049,7 +1049,9 @@ impl<'hir> Map<'hir> {
Some(EntryStructCtor(_, _, _)) => self.expect_item(self.get_parent(id)).span,
Some(EntryLifetime(_, _, lifetime)) => lifetime.span,
Some(EntryGenericParam(_, _, param)) => param.span,
- Some(EntryVisibility(_, _, &Visibility::Restricted { ref path, .. })) => path.span,
+ Some(EntryVisibility(_, _, &Spanned {
+ node: VisibilityRestricted { ref path, .. }, ..
+ })) => path.span,
Some(EntryVisibility(_, _, v)) => bug!("unexpected Visibility {:?}", v),
Some(EntryLocal(_, _, local)) => local.span,
Some(EntryMacroDef(_, macro_def)) => macro_def.span,
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
index d8bf5fe9b6d..6f8981e1e6c 100644
--- a/src/librustc/hir/mod.rs
+++ b/src/librustc/hir/mod.rs
@@ -24,7 +24,7 @@ pub use self::Stmt_::*;
pub use self::Ty_::*;
pub use self::UnOp::*;
pub use self::UnsafeSource::*;
-pub use self::Visibility::{Public, Inherited};
+pub use self::Visibility_::*;
use hir::def::Def;
use hir::def_id::{DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX};
@@ -1929,22 +1929,30 @@ pub struct PolyTraitRef {
pub span: Span,
}
+pub type Visibility = Spanned<Visibility_>;
+
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
-pub enum Visibility {
- Public,
- Crate(CrateSugar),
- Restricted { path: P<Path>, id: NodeId },
- Inherited,
+pub enum Visibility_ {
+ VisibilityPublic,
+ VisibilityCrate(CrateSugar),
+ VisibilityRestricted { path: P<Path>, id: NodeId },
+ VisibilityInherited,
}
-impl Visibility {
+impl Visibility_ {
+ pub fn is_pub(&self) -> bool {
+ match *self {
+ VisibilityPublic => true,
+ _ => false
+ }
+ }
+
pub fn is_pub_restricted(&self) -> bool {
- use self::Visibility::*;
- match self {
- &Public |
- &Inherited => false,
- &Crate(_) |
- &Restricted { .. } => true,
+ match *self {
+ VisibilityPublic |
+ VisibilityInherited => false,
+ VisibilityCrate(..) |
+ VisibilityRestricted { .. } => true,
}
}
}
diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs
index c6f69a84d03..5b52764f08d 100644
--- a/src/librustc/hir/print.rs
+++ b/src/librustc/hir/print.rs
@@ -12,7 +12,7 @@ pub use self::AnnNode::*;
use rustc_target::spec::abi::Abi;
use syntax::ast;
-use syntax::codemap::CodeMap;
+use syntax::codemap::{CodeMap, Spanned};
use syntax::parse::ParseSess;
use syntax::parse::lexer::comments;
use syntax::print::pp::{self, Breaks};
@@ -839,11 +839,11 @@ impl<'a> State<'a> {
}
pub fn print_visibility(&mut self, vis: &hir::Visibility) -> io::Result<()> {
- match *vis {
- hir::Public => self.word_nbsp("pub")?,
- hir::Visibility::Crate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
- hir::Visibility::Crate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
- hir::Visibility::Restricted { ref path, .. } => {
+ match vis.node {
+ hir::VisibilityPublic => self.word_nbsp("pub")?,
+ hir::VisibilityCrate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
+ hir::VisibilityCrate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
+ hir::VisibilityRestricted { ref path, .. } => {
self.s.word("pub(")?;
if path.segments.len() == 1 &&
path.segments[0].ident.name == keywords::Super.name() {
@@ -856,7 +856,7 @@ impl<'a> State<'a> {
}
self.word_nbsp(")")?;
}
- hir::Inherited => ()
+ hir::VisibilityInherited => ()
}
Ok(())
@@ -952,17 +952,18 @@ impl<'a> State<'a> {
self.print_outer_attributes(&ti.attrs)?;
match ti.node {
hir::TraitItemKind::Const(ref ty, default) => {
- self.print_associated_const(ti.ident, &ty, default, &hir::Inherited)?;
+ let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
+ self.print_associated_const(ti.ident, &ty, default, &vis)?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(ref arg_names)) => {
- self.print_method_sig(ti.ident, sig, &ti.generics, &hir::Inherited, arg_names,
- None)?;
+ let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
+ self.print_method_sig(ti.ident, sig, &ti.generics, &vis, arg_names, None)?;
self.s.word(";")?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => {
+ let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
self.head("")?;
- self.print_method_sig(ti.ident, sig, &ti.generics, &hir::Inherited, &[],
- Some(body))?;
+ self.print_method_sig(ti.ident, sig, &ti.generics, &vis, &[], Some(body))?;
self.nbsp()?;
self.end()?; // need to close a box
self.end()?; // need to close a box
@@ -2266,7 +2267,7 @@ impl<'a> State<'a> {
},
name,
&generics,
- &hir::Inherited,
+ &Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited },
arg_names,
None)?;
self.end()
diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs
index d59a20c6522..3170f9efafb 100644
--- a/src/librustc/ich/impls_hir.rs
+++ b/src/librustc/ich/impls_hir.rs
@@ -710,20 +710,20 @@ impl_stable_hash_for!(enum ::syntax::ast::CrateSugar {
PubCrate,
});
-impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility {
+impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility_ {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
- hir::Visibility::Public |
- hir::Visibility::Inherited => {
+ hir::VisibilityPublic |
+ hir::VisibilityInherited => {
// No fields to hash.
}
- hir::Visibility::Crate(sugar) => {
+ hir::VisibilityCrate(sugar) => {
sugar.hash_stable(hcx, hasher);
}
- hir::Visibility::Restricted { ref path, id } => {
+ hir::VisibilityRestricted { ref path, id } => {
hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
id.hash_stable(hcx, hasher);
});
@@ -733,6 +733,8 @@ impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility {
}
}
+impl_stable_hash_for_spanned!(hir::Visibility_);
+
impl<'a> HashStable<StableHashingContext<'a>> for hir::Defaultness {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs
index caf73096ebf..976dee3f7da 100644
--- a/src/librustc/middle/dead.rs
+++ b/src/librustc/middle/dead.rs
@@ -157,7 +157,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
intravisit::walk_item(self, &item);
}
hir::ItemEnum(..) => {
- self.inherited_pub_visibility = item.vis == hir::Public;
+ self.inherited_pub_visibility = item.vis.node.is_pub();
intravisit::walk_item(self, &item);
}
hir::ItemFn(..)
@@ -212,7 +212,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> {
let has_repr_c = self.repr_has_repr_c;
let inherited_pub_visibility = self.inherited_pub_visibility;
let live_fields = def.fields().iter().filter(|f| {
- has_repr_c || inherited_pub_visibility || f.vis == hir::Public
+ has_repr_c || inherited_pub_visibility || f.vis.node.is_pub()
});
self.live_symbols.extend(live_fields.map(|f| f.id));
diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 1f647d811b0..aad60c0247d 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -268,16 +268,16 @@ impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> {
impl Visibility {
pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self {
- match *visibility {
- hir::Public => Visibility::Public,
- hir::Visibility::Crate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
- hir::Visibility::Restricted { ref path, .. } => match path.def {
+ match visibility.node {
+ hir::VisibilityPublic => Visibility::Public,
+ hir::VisibilityCrate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
+ hir::VisibilityRestricted { ref path, .. } => match path.def {
// If there is no resolution, `resolve` will have already reported an error, so
// assume that the visibility is public to avoid reporting more privacy errors.
Def::Err => Visibility::Public,
def => Visibility::Restricted(def.def_id()),
},
- hir::Inherited => {
+ hir::VisibilityInherited => {
Visibility::Restricted(tcx.hir.get_module_parent(id))
}
}
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index b4dc5f9c85b..c5bde8af9cf 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -397,7 +397,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
hir::ItemUnion(..) => "a union",
hir::ItemTrait(.., ref trait_item_refs) => {
// Issue #11592, traits are always considered exported, even when private.
- if it.vis == hir::Visibility::Inherited {
+ if it.vis.node == hir::VisibilityInherited {
self.private_traits.insert(it.id);
for trait_item_ref in trait_item_refs {
self.private_traits.insert(trait_item_ref.id.node_id);
@@ -414,7 +414,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
if let Some(node_id) = cx.tcx.hir.as_local_node_id(real_trait) {
match cx.tcx.hir.find(node_id) {
Some(hir_map::NodeItem(item)) => {
- if item.vis == hir::Visibility::Inherited {
+ if item.vis.node == hir::VisibilityInherited {
for impl_item_ref in impl_item_refs {
self.private_traits.insert(impl_item_ref.id.node_id);
}
@@ -1187,7 +1187,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
let msg = "function is marked #[no_mangle], but not exported";
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
let insertion_span = it.span.shrink_to_lo();
- if it.vis == hir::Visibility::Inherited {
+ if it.vis.node == hir::VisibilityInherited {
err.span_suggestion(insertion_span,
"try making it public",
"pub ".to_owned());
@@ -1218,7 +1218,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
let msg = "static is marked #[no_mangle], but not exported";
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
let insertion_span = it.span.shrink_to_lo();
- if it.vis == hir::Visibility::Inherited {
+ if it.vis.node == hir::VisibilityInherited {
err.span_suggestion(insertion_span,
"try making it public",
"pub ".to_owned());
@@ -1388,7 +1388,7 @@ impl UnreachablePub {
fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId,
vis: &hir::Visibility, span: Span, exportable: bool,
mut applicability: Applicability) {
- if !cx.access_levels.is_reachable(id) && *vis == hir::Visibility::Public {
+ if !cx.access_levels.is_reachable(id) && vis.node.is_pub() {
if span.ctxt().outer().expn_info().is_some() {
applicability = Applicability::MaybeIncorrect;
}
diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs
index 93294075272..196c53970f6 100644
--- a/src/librustc_metadata/encoder.rs
+++ b/src/librustc_metadata/encoder.rs
@@ -40,6 +40,7 @@ use rustc_data_structures::sync::Lrc;
use std::u32;
use syntax::ast::{self, CRATE_NODE_ID};
use syntax::attr;
+use syntax::codemap::Spanned;
use syntax::symbol::keywords;
use syntax_pos::{self, hygiene, FileName, FileMap, Span};
@@ -319,9 +320,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
fn encode_info_for_items(&mut self) -> Index {
let krate = self.tcx.hir.krate();
let mut index = IndexBuilder::new(self);
+ let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityPublic };
index.record(DefId::local(CRATE_DEF_INDEX),
IsolatedEncoder::encode_info_for_mod,
- FromId(CRATE_NODE_ID, (&krate.module, &krate.attrs, &hir::Public)));
+ FromId(CRATE_NODE_ID, (&krate.module, &krate.attrs, &vis)));
let mut visitor = EncodeVisitor { index: index };
krate.visit_all_item_likes(&mut visitor.as_deep_visitor());
for macro_def in &krate.exported_macros {
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
index 2aecbf32ec5..05ee85e92f5 100644
--- a/src/librustc_privacy/lib.rs
+++ b/src/librustc_privacy/lib.rs
@@ -61,7 +61,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PubRestrictedVisitor<'a, 'tcx> {
NestedVisitorMap::All(&self.tcx.hir)
}
fn visit_vis(&mut self, vis: &'tcx hir::Visibility) {
- self.has_pub_restricted = self.has_pub_restricted || vis.is_pub_restricted();
+ self.has_pub_restricted = self.has_pub_restricted || vis.node.is_pub_restricted();
}
}
@@ -162,7 +162,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
hir::ItemTrait(..) | hir::ItemTraitAlias(..) |
hir::ItemExistential(..) |
hir::ItemTy(..) | hir::ItemUnion(..) | hir::ItemUse(..) => {
- if item.vis == hir::Public { self.prev_level } else { None }
+ if item.vis.node.is_pub() { self.prev_level } else { None }
}
};
@@ -181,7 +181,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
}
hir::ItemImpl(.., None, _, ref impl_item_refs) => {
for impl_item_ref in impl_item_refs {
- if impl_item_ref.vis == hir::Public {
+ if impl_item_ref.vis.node.is_pub() {
self.update(impl_item_ref.id.node_id, item_level);
}
}
@@ -201,14 +201,14 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
self.update(def.id(), item_level);
}
for field in def.fields() {
- if field.vis == hir::Public {
+ if field.vis.node.is_pub() {
self.update(field.id, item_level);
}
}
}
hir::ItemForeignMod(ref foreign_mod) => {
for foreign_item in &foreign_mod.items {
- if foreign_item.vis == hir::Public {
+ if foreign_item.vis.node.is_pub() {
self.update(foreign_item.id, item_level);
}
}
@@ -358,7 +358,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
let module_did = ty::DefIdTree::parent(self.tcx, self.tcx.hir.local_def_id(md.id)).unwrap();
let mut module_id = self.tcx.hir.as_local_node_id(module_did).unwrap();
- let level = if md.vis == hir::Public { self.get(module_id) } else { None };
+ let level = if md.vis.node.is_pub() { self.get(module_id) } else { None };
let level = self.update(md.id, level);
if level.is_none() {
return
@@ -1023,7 +1023,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.hir.find(node_id) {
- Some(hir::map::NodeItem(ref item)) => item.vis != hir::Public,
+ Some(hir::map::NodeItem(ref item)) => !item.vis.node.is_pub(),
Some(_) | None => false,
}
} else {
@@ -1046,7 +1046,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
}
fn item_is_public(&self, id: &ast::NodeId, vis: &hir::Visibility) -> bool {
- self.access_levels.is_reachable(*id) || *vis == hir::Public
+ self.access_levels.is_reachable(*id) || vis.node.is_pub()
}
}
@@ -1317,7 +1317,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
}
fn visit_struct_field(&mut self, s: &'tcx hir::StructField) {
- if s.vis == hir::Public || self.in_variant {
+ if s.vis.node.is_pub() || self.in_variant {
intravisit::walk_struct_field(self, s);
}
}
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
index 685c86029b6..414247d22a8 100644
--- a/src/librustc_save_analysis/lib.rs
+++ b/src/librustc_save_analysis/lib.rs
@@ -55,6 +55,7 @@ use std::fs::File;
use std::path::{Path, PathBuf};
use syntax::ast::{self, Attribute, NodeId, PatKind};
+use syntax::codemap::Spanned;
use syntax::parse::lexer::comments::strip_doc_comment_decoration;
use syntax::parse::token;
use syntax::print::pprust;
@@ -631,7 +632,8 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
node: hir::ItemUse(ref path, _),
..
}) |
- Node::NodeVisibility(&hir::Visibility::Restricted { ref path, .. }) => path.def,
+ Node::NodeVisibility(&Spanned {
+ node: hir::VisibilityRestricted { ref path, .. }, .. }) => path.def,
Node::NodeExpr(&hir::Expr {
node: hir::ExprStruct(ref qpath, ..),
diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs
index ae5ca5441ad..3a8ed0ea25f 100644
--- a/src/librustc_typeck/check_unused.rs
+++ b/src/librustc_typeck/check_unused.rs
@@ -39,7 +39,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CheckVisitor<'a, 'tcx> {
fn visit_item(&mut self, item: &hir::Item) {
- if item.vis == hir::Public || item.span.is_dummy() {
+ if item.vis.node.is_pub() || item.span.is_dummy() {
return;
}
if let hir::ItemUse(ref path, _) = item.node {
@@ -214,4 +214,3 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CollectExternCrateVisitor<'a, 'tcx> {
fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) {
}
}
-
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 65babbffffe..7394b6d580c 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -286,7 +286,7 @@ impl Clean<ExternalCrate> for CrateNum {
as_primitive(Def::Mod(cx.tcx.hir.local_def_id(id.id)))
}
hir::ItemUse(ref path, hir::UseKind::Single)
- if item.vis == hir::Visibility::Public => {
+ if item.vis.node.is_pub() => {
as_primitive(path.def).map(|(_, prim, attrs)| {
// Pretend the primitive is local.
(cx.tcx.hir.local_def_id(id.id), prim, attrs)
@@ -328,7 +328,7 @@ impl Clean<ExternalCrate> for CrateNum {
as_keyword(Def::Mod(cx.tcx.hir.local_def_id(id.id)))
}
hir::ItemUse(ref path, hir::UseKind::Single)
- if item.vis == hir::Visibility::Public => {
+ if item.vis.node.is_pub() => {
as_keyword(path.def).map(|(_, prim, attrs)| {
(cx.tcx.hir.local_def_id(id.id), prim, attrs)
})
@@ -3225,11 +3225,11 @@ pub enum Visibility {
impl Clean<Option<Visibility>> for hir::Visibility {
fn clean(&self, cx: &DocContext) -> Option<Visibility> {
- Some(match *self {
- hir::Visibility::Public => Visibility::Public,
- hir::Visibility::Inherited => Visibility::Inherited,
- hir::Visibility::Crate(_) => Visibility::Crate,
- hir::Visibility::Restricted { ref path, .. } => {
+ Some(match self.node {
+ hir::VisibilityPublic => Visibility::Public,
+ hir::VisibilityInherited => Visibility::Inherited,
+ hir::VisibilityCrate(_) => Visibility::Crate,
+ hir::VisibilityRestricted { ref path, .. } => {
let path = path.clean(cx);
let did = register_def(cx, path.def);
Visibility::Restricted(did, path)
@@ -3932,7 +3932,7 @@ impl Clean<Vec<Item>> for doctree::Import {
// forcefully don't inline if this is not public or if the
// #[doc(no_inline)] attribute is present.
// Don't inline doc(hidden) imports so they can be stripped at a later stage.
- let denied = self.vis != hir::Public || self.attrs.iter().any(|a| {
+ let denied = !self.vis.node.is_pub() || self.attrs.iter().any(|a| {
a.name() == "doc" && match a.meta_item_list() {
Some(l) => attr::list_contains_name(&l, "no_inline") ||
attr::list_contains_name(&l, "hidden"),
diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs
index 0807db29976..76c11a4e7cd 100644
--- a/src/librustdoc/doctree.rs
+++ b/src/librustdoc/doctree.rs
@@ -17,6 +17,7 @@ use syntax::ast;
use syntax::ast::{Name, NodeId};
use syntax::attr;
use syntax::ptr::P;
+use syntax::codemap::Spanned;
use syntax_pos::{self, Span};
use rustc::hir;
@@ -53,7 +54,7 @@ impl Module {
Module {
name : name,
id: ast::CRATE_NODE_ID,
- vis: hir::Inherited,
+ vis: Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited },
stab: None,
depr: None,
where_outer: syntax_pos::DUMMY_SP,
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index 6bf1931e468..3a13f61327e 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -15,7 +15,8 @@ use std::mem;
use syntax::ast;
use syntax::attr;
-use syntax_pos::Span;
+use syntax::codemap::Spanned;
+use syntax_pos::{self, Span};
use rustc::hir::map as hir_map;
use rustc::hir::def::Def;
@@ -94,7 +95,8 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
self.module = self.visit_mod_contents(krate.span,
krate.attrs.clone(),
- hir::Public,
+ Spanned { span: syntax_pos::DUMMY_SP,
+ node: hir::VisibilityPublic },
ast::CRATE_NODE_ID,
&krate.module,
None);
@@ -204,7 +206,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
om.id = id;
// Keep track of if there were any private modules in the path.
let orig_inside_public_path = self.inside_public_path;
- self.inside_public_path &= vis == hir::Public;
+ self.inside_public_path &= vis.node.is_pub();
for i in &m.item_ids {
let item = self.cx.tcx.hir.expect_item(i.id);
self.visit_item(item, None, &mut om);
@@ -376,7 +378,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
debug!("Visiting item {:?}", item);
let name = renamed.unwrap_or(item.name);
- if item.vis == hir::Public {
+ if item.vis.node.is_pub() {
let def_id = self.cx.tcx.hir.local_def_id(item.id);
self.store_path(def_id);
}
@@ -387,14 +389,14 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
om.foreigns.push(if self.inlining {
hir::ForeignMod {
abi: fm.abi,
- items: fm.items.iter().filter(|i| i.vis == hir::Public).cloned().collect(),
+ items: fm.items.iter().filter(|i| i.vis.node.is_pub()).cloned().collect(),
}
} else {
fm.clone()
});
}
// If we're inlining, skip private items.
- _ if self.inlining && item.vis != hir::Public => {}
+ _ if self.inlining && !item.vis.node.is_pub() => {}
hir::ItemGlobalAsm(..) => {}
hir::ItemExternCrate(orig_name) => {
let def_id = self.cx.tcx.hir.local_def_id(item.id);
@@ -414,7 +416,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
// If there was a private module in the current path then don't bother inlining
// anything as it will probably be stripped anyway.
- if item.vis == hir::Public && self.inside_public_path {
+ if item.vis.node.is_pub() && self.inside_public_path {
let please_inline = item.attrs.iter().any(|item| {
match item.meta_item_list() {
Some(ref list) if item.check_name("doc") => {
From 104985b8275180ebe8811f8957a93740855995e0 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Wed, 27 Jun 2018 21:23:18 -0700
Subject: [PATCH 3/7] unreachable_pub lint: grab `pub` span from HIR rather
than inferring it
This is a true fix for #50455, superior to the mere bandage offered
in #50476.
---
src/librustc_lint/builtin.rs | 67 +++++++++----------
.../ui/lint/unreachable_pub-pub_crate.stderr | 13 ++--
src/test/ui/lint/unreachable_pub.stderr | 13 ++--
3 files changed, 46 insertions(+), 47 deletions(-)
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index c5bde8af9cf..a9ee1c8f176 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -1386,31 +1386,32 @@ impl LintPass for UnreachablePub {
impl UnreachablePub {
fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId,
- vis: &hir::Visibility, span: Span, exportable: bool,
- mut applicability: Applicability) {
- if !cx.access_levels.is_reachable(id) && vis.node.is_pub() {
- if span.ctxt().outer().expn_info().is_some() {
- applicability = Applicability::MaybeIncorrect;
- }
- let def_span = cx.tcx.sess.codemap().def_span(span);
- let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
- &format!("unreachable `pub` {}", what));
- // We are presuming that visibility is token at start of
- // declaration (can be macro variable rather than literal `pub`)
- let pub_span = cx.tcx.sess.codemap().span_until_char(def_span, ' ');
- let replacement = if cx.tcx.features().crate_visibility_modifier {
- "crate"
- } else {
- "pub(crate)"
- }.to_owned();
- err.span_suggestion_with_applicability(pub_span,
- "consider restricting its visibility",
- replacement,
- applicability);
- if exportable {
- err.help("or consider exporting it for use by other crates");
- }
- err.emit();
+ vis: &hir::Visibility, span: Span, exportable: bool) {
+ let mut applicability = Applicability::MachineApplicable;
+ match vis.node {
+ hir::VisibilityPublic if !cx.access_levels.is_reachable(id) => {
+ if span.ctxt().outer().expn_info().is_some() {
+ applicability = Applicability::MaybeIncorrect;
+ }
+ let def_span = cx.tcx.sess.codemap().def_span(span);
+ let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
+ &format!("unreachable `pub` {}", what));
+ let replacement = if cx.tcx.features().crate_visibility_modifier {
+ "crate"
+ } else {
+ "pub(crate)"
+ }.to_owned();
+
+ err.span_suggestion_with_applicability(vis.span,
+ "consider restricting its visibility",
+ replacement,
+ applicability);
+ if exportable {
+ err.help("or consider exporting it for use by other crates");
+ }
+ err.emit();
+ },
+ _ => {}
}
}
}
@@ -1418,28 +1419,20 @@ impl UnreachablePub {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
- let applicability = match item.node {
- // suggestion span-manipulation is inadequate for `pub use
- // module::{item}` (Issue #50455)
- hir::ItemUse(..) => Applicability::MaybeIncorrect,
- _ => Applicability::MachineApplicable,
- };
- self.perform_lint(cx, "item", item.id, &item.vis, item.span, true, applicability);
+ self.perform_lint(cx, "item", item.id, &item.vis, item.span, true);
}
fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) {
self.perform_lint(cx, "item", foreign_item.id, &foreign_item.vis,
- foreign_item.span, true, Applicability::MachineApplicable);
+ foreign_item.span, true);
}
fn check_struct_field(&mut self, cx: &LateContext, field: &hir::StructField) {
- self.perform_lint(cx, "field", field.id, &field.vis, field.span, false,
- Applicability::MachineApplicable);
+ self.perform_lint(cx, "field", field.id, &field.vis, field.span, false);
}
fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) {
- self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false,
- Applicability::MachineApplicable);
+ self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false);
}
}
diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.stderr b/src/test/ui/lint/unreachable_pub-pub_crate.stderr
index 2948deb2300..1cbfbd21125 100644
--- a/src/test/ui/lint/unreachable_pub-pub_crate.stderr
+++ b/src/test/ui/lint/unreachable_pub-pub_crate.stderr
@@ -17,7 +17,9 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:27:24
|
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
- | ^^^^ help: consider restricting its visibility: `pub(crate)`
+ | --- ^^^^
+ | |
+ | help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates
@@ -121,12 +123,13 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:50:47
|
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
- | -----------^^^^^^^^^^^^^
- | |
- | help: consider restricting its visibility: `pub(crate)`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
LL | }
LL | define_empty_struct_with_visibility!(pub, Fluorine);
- | ---------------------------------------------------- in this macro invocation
+ | ----------------------------------------------------
+ | | |
+ | | help: consider restricting its visibility: `pub(crate)`
+ | in this macro invocation
|
= help: or consider exporting it for use by other crates
diff --git a/src/test/ui/lint/unreachable_pub.stderr b/src/test/ui/lint/unreachable_pub.stderr
index ad88c55d540..25046055aa0 100644
--- a/src/test/ui/lint/unreachable_pub.stderr
+++ b/src/test/ui/lint/unreachable_pub.stderr
@@ -17,7 +17,9 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub.rs:22:24
|
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
- | ^^^^ help: consider restricting its visibility: `crate`
+ | --- ^^^^
+ | |
+ | help: consider restricting its visibility: `crate`
|
= help: or consider exporting it for use by other crates
@@ -121,12 +123,13 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub.rs:45:47
|
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
- | -----------^^^^^^^^^^^^^
- | |
- | help: consider restricting its visibility: `crate`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
LL | }
LL | define_empty_struct_with_visibility!(pub, Fluorine);
- | ---------------------------------------------------- in this macro invocation
+ | ----------------------------------------------------
+ | | |
+ | | help: consider restricting its visibility: `crate`
+ | in this macro invocation
|
= help: or consider exporting it for use by other crates
From 53307473fdaebde701a54fc58b307d142b38b569 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Wed, 27 Jun 2018 22:30:23 -0700
Subject: [PATCH 4/7] private no-mangle lints: issue suggestion for restricted
visibility
This is probably quite a lot less likely to come up in practice than the
"inherited" (no visibility keyword) case, but now that we have
visibility spans in the HIR, we can do this, and it presumably doesn't
hurt to be exhaustive. (Who can say but that the attention to detail
just might knock someone's socks off, someday, somewhere?)
This is inspired by #47383.
---
src/librustc_lint/builtin.rs | 41 +++++++++++++++++------------
src/test/ui/lint/suggestions.rs | 6 +++++
src/test/ui/lint/suggestions.stderr | 28 +++++++++++++++-----
3 files changed, 52 insertions(+), 23 deletions(-)
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index a9ee1c8f176..f4159002eb3 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -1177,6 +1177,23 @@ impl LintPass for InvalidNoMangleItems {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
+ let suggest_make_pub = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
+ let suggestion = match vis.node {
+ hir::VisibilityInherited => {
+ // inherited visibility is empty span at item start; need an extra space
+ Some("pub ".to_owned())
+ },
+ hir::VisibilityRestricted { .. } |
+ hir::VisibilityCrate(_) => {
+ Some("pub".to_owned())
+ },
+ hir::VisibilityPublic => None
+ };
+ if let Some(replacement) = suggestion {
+ err.span_suggestion(vis.span, "try making it public", replacement);
+ }
+ };
+
match it.node {
hir::ItemFn(.., ref generics, _) => {
if let Some(no_mangle_attr) = attr::find_by_name(&it.attrs, "no_mangle") {
@@ -1186,12 +1203,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
if !cx.access_levels.is_reachable(it.id) {
let msg = "function is marked #[no_mangle], but not exported";
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
- let insertion_span = it.span.shrink_to_lo();
- if it.vis.node == hir::VisibilityInherited {
- err.span_suggestion(insertion_span,
- "try making it public",
- "pub ".to_owned());
- }
+ suggest_make_pub(&it.vis, &mut err);
err.emit();
}
for param in &generics.params {
@@ -1214,17 +1226,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
}
hir::ItemStatic(..) => {
if attr::contains_name(&it.attrs, "no_mangle") &&
- !cx.access_levels.is_reachable(it.id) {
- let msg = "static is marked #[no_mangle], but not exported";
- let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
- let insertion_span = it.span.shrink_to_lo();
- if it.vis.node == hir::VisibilityInherited {
- err.span_suggestion(insertion_span,
- "try making it public",
- "pub ".to_owned());
- }
- err.emit();
- }
+ !cx.access_levels.is_reachable(it.id) {
+ let msg = "static is marked #[no_mangle], but not exported";
+ let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
+ suggest_make_pub(&it.vis, &mut err);
+ err.emit();
+ }
}
hir::ItemConst(..) => {
if attr::contains_name(&it.attrs, "no_mangle") {
diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs
index e35675eacd8..6c767bca74a 100644
--- a/src/test/ui/lint/suggestions.rs
+++ b/src/test/ui/lint/suggestions.rs
@@ -34,6 +34,12 @@ mod badlands {
//~^ WARN static is marked
#[no_mangle] pub fn val_jean() {}
//~^ WARN function is marked
+
+ // ... but we can suggest just-`pub` instead of restricted
+ #[no_mangle] pub(crate) static VETAR: bool = true;
+ //~^ WARN static is marked
+ #[no_mangle] pub(crate) fn crossfield() {}
+ //~^ WARN function is marked
}
struct Equinox {
diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr
index 84a2e4a91ec..adb4b8eb67d 100644
--- a/src/test/ui/lint/suggestions.stderr
+++ b/src/test/ui/lint/suggestions.stderr
@@ -1,5 +1,5 @@
warning: unnecessary parentheses around assigned value
- --> $DIR/suggestions.rs:48:21
+ --> $DIR/suggestions.rs:54:21
|
LL | let mut a = (1); // should suggest no `mut`, no parens
| ^^^ help: remove these parentheses
@@ -11,7 +11,7 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
| ^^^^^^^^^^^^^
warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
- --> $DIR/suggestions.rs:43:1
+ --> $DIR/suggestions.rs:49:1
|
LL | #[no_debug] // should suggest removal of deprecated attribute
| ^^^^^^^^^^^ help: remove this attribute
@@ -19,7 +19,7 @@ LL | #[no_debug] // should suggest removal of deprecated attribute
= note: #[warn(deprecated)] on by default
warning: variable does not need to be mutable
- --> $DIR/suggestions.rs:48:13
+ --> $DIR/suggestions.rs:54:13
|
LL | let mut a = (1); // should suggest no `mut`, no parens
| ----^
@@ -33,7 +33,7 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
| ^^^^^^^^^^
warning: variable does not need to be mutable
- --> $DIR/suggestions.rs:52:13
+ --> $DIR/suggestions.rs:58:13
|
LL | let mut
| _____________^
@@ -96,8 +96,24 @@ warning: function is marked #[no_mangle], but not exported
LL | #[no_mangle] pub fn val_jean() {}
| ^^^^^^^^^^^^^^^^^^^^
+warning: static is marked #[no_mangle], but not exported
+ --> $DIR/suggestions.rs:39:18
+ |
+LL | #[no_mangle] pub(crate) static VETAR: bool = true;
+ | ----------^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ | |
+ | help: try making it public: `pub`
+
+warning: function is marked #[no_mangle], but not exported
+ --> $DIR/suggestions.rs:41:18
+ |
+LL | #[no_mangle] pub(crate) fn crossfield() {}
+ | ----------^^^^^^^^^^^^^^^^^^^
+ | |
+ | help: try making it public: `pub`
+
warning: denote infinite loops with `loop { ... }`
- --> $DIR/suggestions.rs:46:5
+ --> $DIR/suggestions.rs:52:5
|
LL | while true { // should suggest `loop`
| ^^^^^^^^^^ help: use `loop`
@@ -105,7 +121,7 @@ LL | while true { // should suggest `loop`
= note: #[warn(while_true)] on by default
warning: the `warp_factor:` in this pattern is redundant
- --> $DIR/suggestions.rs:57:23
+ --> $DIR/suggestions.rs:63:23
|
LL | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
| ------------^^^^^^^^^^^^
From 8d1cbb018e7643e9e8d1d25b8b7de8a2b0671f2d Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Wed, 27 Jun 2018 22:50:24 -0700
Subject: [PATCH 5/7] private no-mangle lints: help hint note if visibility
modifier is `pub`
If the item is `pub`, one imagines users being confused as to why it's
not reachable/exported; a code suggestion is beyond our local knowledge
here, but we can at least offer a prose hint. (Thanks to Vadim
Petrochenkov for shooting down the present author's original bad idea
for the note text.)
While we're here, use proper HELP expectations instead of ad hoc
comments to communicate (and now, enforce) the expected suggestions in
test/ui/lint/suggestions.rs.
---
src/librustc_lint/builtin.rs | 11 ++++---
src/test/ui/lint/suggestions.rs | 28 ++++++++++++-----
src/test/ui/lint/suggestions.stderr | 47 ++++++++++++++++-------------
3 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index f4159002eb3..96dcb458303 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -1177,7 +1177,7 @@ impl LintPass for InvalidNoMangleItems {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
- let suggest_make_pub = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
+ let suggest_export = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
let suggestion = match vis.node {
hir::VisibilityInherited => {
// inherited visibility is empty span at item start; need an extra space
@@ -1187,7 +1187,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
hir::VisibilityCrate(_) => {
Some("pub".to_owned())
},
- hir::VisibilityPublic => None
+ hir::VisibilityPublic => {
+ err.help("try exporting the item with a `pub use` statement");
+ None
+ }
};
if let Some(replacement) = suggestion {
err.span_suggestion(vis.span, "try making it public", replacement);
@@ -1203,7 +1206,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
if !cx.access_levels.is_reachable(it.id) {
let msg = "function is marked #[no_mangle], but not exported";
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
- suggest_make_pub(&it.vis, &mut err);
+ suggest_export(&it.vis, &mut err);
err.emit();
}
for param in &generics.params {
@@ -1229,7 +1232,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
!cx.access_levels.is_reachable(it.id) {
let msg = "static is marked #[no_mangle], but not exported";
let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
- suggest_make_pub(&it.vis, &mut err);
+ suggest_export(&it.vis, &mut err);
err.emit();
}
}
diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs
index 6c767bca74a..4da2700cb9f 100644
--- a/src/test/ui/lint/suggestions.rs
+++ b/src/test/ui/lint/suggestions.rs
@@ -13,18 +13,22 @@
#![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896
#![feature(no_debug)]
-#[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub`
+#[no_mangle] static SHENZHOU: usize = 1;
//~^ WARN static is marked #[no_mangle]
-#[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const`
+//~| HELP try making it public
+#[no_mangle] const DISCOVERY: usize = 1;
//~^ ERROR const items should never be #[no_mangle]
+//~| HELP try a static value
-#[no_mangle] // should suggest removal (generics can't be no-mangle)
+#[no_mangle]
+//~^ HELP remove this attribute
pub fn defiant<T>(_t: T) {}
//~^ WARN functions generic over types must be mangled
#[no_mangle]
-fn rio_grande() {} // should suggest `pub`
+fn rio_grande() {}
//~^ WARN function is marked
+//~| HELP try making it public
mod badlands {
// The private-no-mangle lints shouldn't suggest inserting `pub` when the
@@ -32,14 +36,18 @@ mod badlands {
// private module). (Issue #47383)
#[no_mangle] pub static DAUNTLESS: bool = true;
//~^ WARN static is marked
+ //~| HELP try exporting the item with a `pub use` statement
#[no_mangle] pub fn val_jean() {}
//~^ WARN function is marked
+ //~| HELP try exporting the item with a `pub use` statement
// ... but we can suggest just-`pub` instead of restricted
#[no_mangle] pub(crate) static VETAR: bool = true;
//~^ WARN static is marked
+ //~| HELP try making it public
#[no_mangle] pub(crate) fn crossfield() {}
//~^ WARN function is marked
+ //~| HELP try making it public
}
struct Equinox {
@@ -48,20 +56,26 @@ struct Equinox {
#[no_debug] // should suggest removal of deprecated attribute
//~^ WARN deprecated
+//~| HELP remove this attribute
fn main() {
- while true { // should suggest `loop`
+ while true {
//~^ WARN denote infinite loops
- let mut a = (1); // should suggest no `mut`, no parens
+ //~| HELP use `loop`
+ let mut a = (1);
//~^ WARN does not need to be mutable
+ //~| HELP remove this `mut`
//~| WARN unnecessary parentheses
+ //~| HELP remove these parentheses
// the line after `mut` has a `\t` at the beginning, this is on purpose
let mut
b = 1;
//~^^ WARN does not need to be mutable
+ //~| HELP remove this `mut`
let d = Equinox { warp_factor: 9.975 };
match d {
- Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
+ Equinox { warp_factor: warp_factor } => {}
//~^ WARN this pattern is redundant
+ //~| HELP remove this
}
println!("{} {}", a, b);
}
diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr
index adb4b8eb67d..8e5dac8be78 100644
--- a/src/test/ui/lint/suggestions.stderr
+++ b/src/test/ui/lint/suggestions.stderr
@@ -1,7 +1,7 @@
warning: unnecessary parentheses around assigned value
- --> $DIR/suggestions.rs:54:21
+ --> $DIR/suggestions.rs:64:21
|
-LL | let mut a = (1); // should suggest no `mut`, no parens
+LL | let mut a = (1);
| ^^^ help: remove these parentheses
|
note: lint level defined here
@@ -11,7 +11,7 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
| ^^^^^^^^^^^^^
warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
- --> $DIR/suggestions.rs:49:1
+ --> $DIR/suggestions.rs:57:1
|
LL | #[no_debug] // should suggest removal of deprecated attribute
| ^^^^^^^^^^^ help: remove this attribute
@@ -19,9 +19,9 @@ LL | #[no_debug] // should suggest removal of deprecated attribute
= note: #[warn(deprecated)] on by default
warning: variable does not need to be mutable
- --> $DIR/suggestions.rs:54:13
+ --> $DIR/suggestions.rs:64:13
|
-LL | let mut a = (1); // should suggest no `mut`, no parens
+LL | let mut a = (1);
| ----^
| |
| help: remove this `mut`
@@ -33,7 +33,7 @@ LL | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issu
| ^^^^^^^^^^
warning: variable does not need to be mutable
- --> $DIR/suggestions.rs:58:13
+ --> $DIR/suggestions.rs:70:13
|
LL | let mut
| _____________^
@@ -47,7 +47,7 @@ LL | || b = 1;
warning: static is marked #[no_mangle], but not exported
--> $DIR/suggestions.rs:16:14
|
-LL | #[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub`
+LL | #[no_mangle] static SHENZHOU: usize = 1;
| -^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: try making it public: `pub`
@@ -55,9 +55,9 @@ LL | #[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub`
= note: #[warn(private_no_mangle_statics)] on by default
error: const items should never be #[no_mangle]
- --> $DIR/suggestions.rs:18:14
+ --> $DIR/suggestions.rs:19:14
|
-LL | #[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const`
+LL | #[no_mangle] const DISCOVERY: usize = 1;
| -----^^^^^^^^^^^^^^^^^^^^^^
| |
| help: try a static value: `pub static`
@@ -65,19 +65,20 @@ LL | #[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rat
= note: #[deny(no_mangle_const_items)] on by default
warning: functions generic over types must be mangled
- --> $DIR/suggestions.rs:22:1
+ --> $DIR/suggestions.rs:25:1
|
-LL | #[no_mangle] // should suggest removal (generics can't be no-mangle)
+LL | #[no_mangle]
| ------------ help: remove this attribute
+LL | //~^ HELP remove this attribute
LL | pub fn defiant<T>(_t: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(no_mangle_generic_items)] on by default
warning: function is marked #[no_mangle], but not exported
- --> $DIR/suggestions.rs:26:1
+ --> $DIR/suggestions.rs:29:1
|
-LL | fn rio_grande() {} // should suggest `pub`
+LL | fn rio_grande() {}
| -^^^^^^^^^^^^^^^^^
| |
| help: try making it public: `pub`
@@ -85,19 +86,23 @@ LL | fn rio_grande() {} // should suggest `pub`
= note: #[warn(private_no_mangle_fns)] on by default
warning: static is marked #[no_mangle], but not exported
- --> $DIR/suggestions.rs:33:18
+ --> $DIR/suggestions.rs:37:18
|
LL | #[no_mangle] pub static DAUNTLESS: bool = true;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: try exporting the item with a `pub use` statement
warning: function is marked #[no_mangle], but not exported
- --> $DIR/suggestions.rs:35:18
+ --> $DIR/suggestions.rs:40:18
|
LL | #[no_mangle] pub fn val_jean() {}
| ^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: try exporting the item with a `pub use` statement
warning: static is marked #[no_mangle], but not exported
- --> $DIR/suggestions.rs:39:18
+ --> $DIR/suggestions.rs:45:18
|
LL | #[no_mangle] pub(crate) static VETAR: bool = true;
| ----------^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -105,7 +110,7 @@ LL | #[no_mangle] pub(crate) static VETAR: bool = true;
| help: try making it public: `pub`
warning: function is marked #[no_mangle], but not exported
- --> $DIR/suggestions.rs:41:18
+ --> $DIR/suggestions.rs:48:18
|
LL | #[no_mangle] pub(crate) fn crossfield() {}
| ----------^^^^^^^^^^^^^^^^^^^
@@ -113,17 +118,17 @@ LL | #[no_mangle] pub(crate) fn crossfield() {}
| help: try making it public: `pub`
warning: denote infinite loops with `loop { ... }`
- --> $DIR/suggestions.rs:52:5
+ --> $DIR/suggestions.rs:61:5
|
-LL | while true { // should suggest `loop`
+LL | while true {
| ^^^^^^^^^^ help: use `loop`
|
= note: #[warn(while_true)] on by default
warning: the `warp_factor:` in this pattern is redundant
- --> $DIR/suggestions.rs:63:23
+ --> $DIR/suggestions.rs:76:23
|
-LL | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
+LL | Equinox { warp_factor: warp_factor } => {}
| ------------^^^^^^^^^^^^
| |
| help: remove this
From c2d44b2286ecf84103e2c66237f578212cd9d8fe Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Sat, 30 Jun 2018 22:08:27 -0700
Subject: [PATCH 6/7] in which the private/restricted-in-public error messaging
gets specific
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
April 2016's Issue #33174 called out the E0446 diagnostics as
confusing. While adding the name of the restricted type to the message
(548e681f) clarified matters somewhat, Esteban Küber pointed out that we
could stand to place a secondary span on the restricted type.
Here, we differentiate between crate-visible, truly private, and
otherwise restricted types, and place a secondary span specifically on
the visibility modifier of the restricted type's declaration (which we
can do now that HIR visibilities have spans!).
At long last, this resolves #33174.
---
src/librustc_privacy/lib.rs | 19 +++++++---
src/test/ui/error-codes/E0446.stderr | 3 ++
...174-restricted-type-in-public-interface.rs | 38 +++++++++++++++++++
...restricted-type-in-public-interface.stderr | 30 +++++++++++++++
4 files changed, 84 insertions(+), 6 deletions(-)
create mode 100644 src/test/ui/pub/issue-33174-restricted-type-in-public-interface.rs
create mode 100644 src/test/ui/pub/issue-33174-restricted-type-in-public-interface.stderr
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
index 05ee85e92f5..dafb89be5fd 100644
--- a/src/librustc_privacy/lib.rs
+++ b/src/librustc_privacy/lib.rs
@@ -1456,29 +1456,36 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'
if let Some(def_id) = ty_def_id {
// Non-local means public (private items can't leave their crate, modulo bugs)
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
- let vis = match self.tcx.hir.find(node_id) {
+ let hir_vis = match self.tcx.hir.find(node_id) {
Some(hir::map::NodeItem(item)) => &item.vis,
Some(hir::map::NodeForeignItem(item)) => &item.vis,
_ => bug!("expected item of foreign item"),
};
- let vis = ty::Visibility::from_hir(vis, node_id, self.tcx);
+ let vis = ty::Visibility::from_hir(hir_vis, node_id, self.tcx);
if !vis.is_at_least(self.min_visibility, self.tcx) {
self.min_visibility = vis;
}
if !vis.is_at_least(self.required_visibility, self.tcx) {
+ let vis_adj = match hir_vis.node {
+ hir::VisibilityCrate(_) => "crate-visible",
+ hir::VisibilityRestricted { .. } => "restricted",
+ _ => "private"
+ };
+
if self.has_pub_restricted || self.has_old_errors || self.in_assoc_ty {
let mut err = struct_span_err!(self.tcx.sess, self.span, E0446,
- "private type `{}` in public interface", ty);
- err.span_label(self.span, "can't leak private type");
+ "{} type `{}` in public interface", vis_adj, ty);
+ err.span_label(self.span, format!("can't leak {} type", vis_adj));
+ err.span_label(hir_vis.span, format!("`{}` declared as {}", ty, vis_adj));
err.emit();
} else {
self.tcx.lint_node(lint::builtin::PRIVATE_IN_PUBLIC,
node_id,
self.span,
- &format!("private type `{}` in public \
- interface (error E0446)", ty));
+ &format!("{} type `{}` in public \
+ interface (error E0446)", vis_adj, ty));
}
}
}
diff --git a/src/test/ui/error-codes/E0446.stderr b/src/test/ui/error-codes/E0446.stderr
index bb5ae494d6c..6c7f3785464 100644
--- a/src/test/ui/error-codes/E0446.stderr
+++ b/src/test/ui/error-codes/E0446.stderr
@@ -1,6 +1,9 @@
error[E0446]: private type `Foo::Bar` in public interface
--> $DIR/E0446.rs:14:5
|
+LL | struct Bar(u32);
+ | - `Foo::Bar` declared as private
+LL |
LL | / pub fn bar() -> Bar { //~ ERROR E0446
LL | | Bar(0)
LL | | }
diff --git a/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.rs b/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.rs
new file mode 100644
index 00000000000..ec3f48f0347
--- /dev/null
+++ b/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.rs
@@ -0,0 +1,38 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(non_camel_case_types)] // genus is always capitalized
+
+pub(crate) struct Snail;
+//~^ NOTE `Snail` declared as crate-visible
+
+mod sea {
+ pub(super) struct Turtle;
+ //~^ NOTE `sea::Turtle` declared as restricted
+}
+
+struct Tortoise;
+//~^ NOTE `Tortoise` declared as private
+
+pub struct Shell<T> {
+ pub(crate) creature: T,
+}
+
+pub type Helix_pomatia = Shell<Snail>;
+//~^ ERROR crate-visible type `Snail` in public interface
+//~| NOTE can't leak crate-visible type
+pub type Dermochelys_coriacea = Shell<sea::Turtle>;
+//~^ ERROR restricted type `sea::Turtle` in public interface
+//~| NOTE can't leak restricted type
+pub type Testudo_graeca = Shell<Tortoise>;
+//~^ ERROR private type `Tortoise` in public interface
+//~| NOTE can't leak private type
+
+fn main() {}
diff --git a/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.stderr b/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.stderr
new file mode 100644
index 00000000000..b35a12f999c
--- /dev/null
+++ b/src/test/ui/pub/issue-33174-restricted-type-in-public-interface.stderr
@@ -0,0 +1,30 @@
+error[E0446]: crate-visible type `Snail` in public interface
+ --> $DIR/issue-33174-restricted-type-in-public-interface.rs:28:1
+ |
+LL | pub(crate) struct Snail;
+ | ---------- `Snail` declared as crate-visible
+...
+LL | pub type Helix_pomatia = Shell<Snail>;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak crate-visible type
+
+error[E0446]: restricted type `sea::Turtle` in public interface
+ --> $DIR/issue-33174-restricted-type-in-public-interface.rs:31:1
+ |
+LL | pub(super) struct Turtle;
+ | ---------- `sea::Turtle` declared as restricted
+...
+LL | pub type Dermochelys_coriacea = Shell<sea::Turtle>;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak restricted type
+
+error[E0446]: private type `Tortoise` in public interface
+ --> $DIR/issue-33174-restricted-type-in-public-interface.rs:34:1
+ |
+LL | struct Tortoise;
+ | - `Tortoise` declared as private
+...
+LL | pub type Testudo_graeca = Shell<Tortoise>;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak private type
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0446`.
From 43a0a65fa2d812c0e48e6cc60a985a4bf47bff57 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Sun, 1 Jul 2018 11:05:10 -0700
Subject: [PATCH 7/7] call it `hir::VisibilityKind` instead of
`hir::Visibility_:*`
It was pointed out in review that the glob-exported
underscore-suffixed convention for `Spanned` HIR nodes is no longer
preferred: see February 2016's #31487 for AST's migration away from
this style towards properly namespaced NodeKind enums.
This concerns #51968.
---
src/librustc/hir/intravisit.rs | 2 +-
src/librustc/hir/lowering.rs | 32 +++++++++++++++----------------
src/librustc/hir/map/collector.rs | 8 ++++----
src/librustc/hir/map/mod.rs | 2 +-
src/librustc/hir/mod.rs | 25 ++++++++++++------------
src/librustc/hir/print.rs | 22 ++++++++++++---------
src/librustc/ich/impls_hir.rs | 12 ++++++------
src/librustc/ty/mod.rs | 8 ++++----
src/librustc_lint/builtin.rs | 14 +++++++-------
src/librustc_metadata/encoder.rs | 2 +-
src/librustc_privacy/lib.rs | 4 ++--
src/librustc_save_analysis/lib.rs | 2 +-
src/librustdoc/clean/mod.rs | 8 ++++----
src/librustdoc/doctree.rs | 2 +-
src/librustdoc/visit_ast.rs | 2 +-
15 files changed, 74 insertions(+), 71 deletions(-)
diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs
index 7173f670cd5..60e944e5aff 100644
--- a/src/librustc/hir/intravisit.rs
+++ b/src/librustc/hir/intravisit.rs
@@ -1104,7 +1104,7 @@ pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
}
pub fn walk_vis<'v, V: Visitor<'v>>(visitor: &mut V, vis: &'v Visibility) {
- if let VisibilityRestricted { ref path, id } = vis.node {
+ if let VisibilityKind::Restricted { ref path, id } = vis.node {
visitor.visit_id(id);
visitor.visit_path(path, id)
}
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 4db12c95168..7219db27834 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -1285,7 +1285,7 @@ impl<'a> LoweringContext<'a> {
name: keywords::Invalid.name(),
attrs: Default::default(),
node: exist_ty_item_kind,
- vis: respan(span.shrink_to_lo(), hir::VisibilityInherited),
+ vis: respan(span.shrink_to_lo(), hir::VisibilityKind::Inherited),
span: exist_ty_span,
};
@@ -2771,11 +2771,11 @@ impl<'a> LoweringContext<'a> {
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
let item = hir::ItemUse(P(path), hir::UseKind::Single);
let vis_kind = match vis.node {
- hir::VisibilityPublic => hir::VisibilityPublic,
- hir::VisibilityCrate(sugar) => hir::VisibilityCrate(sugar),
- hir::VisibilityInherited => hir::VisibilityInherited,
- hir::VisibilityRestricted { ref path, id: _ } => {
- hir::VisibilityRestricted {
+ hir::VisibilityKind::Public => hir::VisibilityKind::Public,
+ hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
+ hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
+ hir::VisibilityKind::Restricted { ref path, id: _ } => {
+ hir::VisibilityKind::Restricted {
path: path.clone(),
// We are allocating a new NodeId here
id: this.next_id().node_id,
@@ -2844,11 +2844,11 @@ impl<'a> LoweringContext<'a> {
self.with_hir_id_owner(new_id, |this| {
let vis_kind = match vis.node {
- hir::VisibilityPublic => hir::VisibilityPublic,
- hir::VisibilityCrate(sugar) => hir::VisibilityCrate(sugar),
- hir::VisibilityInherited => hir::VisibilityInherited,
- hir::VisibilityRestricted { ref path, id: _ } => {
- hir::VisibilityRestricted {
+ hir::VisibilityKind::Public => hir::VisibilityKind::Public,
+ hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
+ hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
+ hir::VisibilityKind::Restricted { ref path, id: _ } => {
+ hir::VisibilityKind::Restricted {
path: path.clone(),
// We are allocating a new NodeId here
id: this.next_id().node_id,
@@ -2876,7 +2876,7 @@ impl<'a> LoweringContext<'a> {
// the stability of `use a::{};`, to avoid it showing up as
// a re-export by accident when `pub`, e.g. in documentation.
let path = P(self.lower_path(id, &prefix, ParamMode::Explicit));
- *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityInherited);
+ *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited);
hir::ItemUse(path, hir::UseKind::ListStem)
}
}
@@ -4277,9 +4277,9 @@ impl<'a> LoweringContext<'a> {
explicit_owner: Option<NodeId>,
) -> hir::Visibility {
let node = match v.node {
- VisibilityKind::Public => hir::VisibilityPublic,
- VisibilityKind::Crate(sugar) => hir::VisibilityCrate(sugar),
- VisibilityKind::Restricted { ref path, id } => hir::VisibilityRestricted {
+ VisibilityKind::Public => hir::VisibilityKind::Public,
+ VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
+ VisibilityKind::Restricted { ref path, id } => hir::VisibilityKind::Restricted {
path: P(self.lower_path(id, path, ParamMode::Explicit)),
id: if let Some(owner) = explicit_owner {
self.lower_node_id_with_owner(id, owner).node_id
@@ -4287,7 +4287,7 @@ impl<'a> LoweringContext<'a> {
self.lower_node_id(id).node_id
},
},
- VisibilityKind::Inherited => hir::VisibilityInherited,
+ VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
};
respan(v.span, node)
}
diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs
index 55ad73515c0..3cc25bfd2d4 100644
--- a/src/librustc/hir/map/collector.rs
+++ b/src/librustc/hir/map/collector.rs
@@ -459,10 +459,10 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
fn visit_vis(&mut self, visibility: &'hir Visibility) {
match visibility.node {
- VisibilityPublic |
- VisibilityCrate(_) |
- VisibilityInherited => {}
- VisibilityRestricted { id, .. } => {
+ VisibilityKind::Public |
+ VisibilityKind::Crate(_) |
+ VisibilityKind::Inherited => {}
+ VisibilityKind::Restricted { id, .. } => {
self.insert(id, NodeVisibility(visibility));
self.with_parent(id, |this| {
intravisit::walk_vis(this, visibility);
diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
index a5c0a5e33f7..08a130f049b 100644
--- a/src/librustc/hir/map/mod.rs
+++ b/src/librustc/hir/map/mod.rs
@@ -1050,7 +1050,7 @@ impl<'hir> Map<'hir> {
Some(EntryLifetime(_, _, lifetime)) => lifetime.span,
Some(EntryGenericParam(_, _, param)) => param.span,
Some(EntryVisibility(_, _, &Spanned {
- node: VisibilityRestricted { ref path, .. }, ..
+ node: VisibilityKind::Restricted { ref path, .. }, ..
})) => path.span,
Some(EntryVisibility(_, _, v)) => bug!("unexpected Visibility {:?}", v),
Some(EntryLocal(_, _, local)) => local.span,
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
index 6f8981e1e6c..b0b81316148 100644
--- a/src/librustc/hir/mod.rs
+++ b/src/librustc/hir/mod.rs
@@ -24,7 +24,6 @@ pub use self::Stmt_::*;
pub use self::Ty_::*;
pub use self::UnOp::*;
pub use self::UnsafeSource::*;
-pub use self::Visibility_::*;
use hir::def::Def;
use hir::def_id::{DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX};
@@ -1929,30 +1928,30 @@ pub struct PolyTraitRef {
pub span: Span,
}
-pub type Visibility = Spanned<Visibility_>;
+pub type Visibility = Spanned<VisibilityKind>;
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
-pub enum Visibility_ {
- VisibilityPublic,
- VisibilityCrate(CrateSugar),
- VisibilityRestricted { path: P<Path>, id: NodeId },
- VisibilityInherited,
+pub enum VisibilityKind {
+ Public,
+ Crate(CrateSugar),
+ Restricted { path: P<Path>, id: NodeId },
+ Inherited,
}
-impl Visibility_ {
+impl VisibilityKind {
pub fn is_pub(&self) -> bool {
match *self {
- VisibilityPublic => true,
+ VisibilityKind::Public => true,
_ => false
}
}
pub fn is_pub_restricted(&self) -> bool {
match *self {
- VisibilityPublic |
- VisibilityInherited => false,
- VisibilityCrate(..) |
- VisibilityRestricted { .. } => true,
+ VisibilityKind::Public |
+ VisibilityKind::Inherited => false,
+ VisibilityKind::Crate(..) |
+ VisibilityKind::Restricted { .. } => true,
}
}
}
diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs
index 5b52764f08d..255009c94c6 100644
--- a/src/librustc/hir/print.rs
+++ b/src/librustc/hir/print.rs
@@ -840,10 +840,10 @@ impl<'a> State<'a> {
pub fn print_visibility(&mut self, vis: &hir::Visibility) -> io::Result<()> {
match vis.node {
- hir::VisibilityPublic => self.word_nbsp("pub")?,
- hir::VisibilityCrate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
- hir::VisibilityCrate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
- hir::VisibilityRestricted { ref path, .. } => {
+ hir::VisibilityKind::Public => self.word_nbsp("pub")?,
+ hir::VisibilityKind::Crate(ast::CrateSugar::JustCrate) => self.word_nbsp("crate")?,
+ hir::VisibilityKind::Crate(ast::CrateSugar::PubCrate) => self.word_nbsp("pub(crate)")?,
+ hir::VisibilityKind::Restricted { ref path, .. } => {
self.s.word("pub(")?;
if path.segments.len() == 1 &&
path.segments[0].ident.name == keywords::Super.name() {
@@ -856,7 +856,7 @@ impl<'a> State<'a> {
}
self.word_nbsp(")")?;
}
- hir::VisibilityInherited => ()
+ hir::VisibilityKind::Inherited => ()
}
Ok(())
@@ -952,16 +952,19 @@ impl<'a> State<'a> {
self.print_outer_attributes(&ti.attrs)?;
match ti.node {
hir::TraitItemKind::Const(ref ty, default) => {
- let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
+ let vis = Spanned { span: syntax_pos::DUMMY_SP,
+ node: hir::VisibilityKind::Inherited };
self.print_associated_const(ti.ident, &ty, default, &vis)?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(ref arg_names)) => {
- let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
+ let vis = Spanned { span: syntax_pos::DUMMY_SP,
+ node: hir::VisibilityKind::Inherited };
self.print_method_sig(ti.ident, sig, &ti.generics, &vis, arg_names, None)?;
self.s.word(";")?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => {
- let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited };
+ let vis = Spanned { span: syntax_pos::DUMMY_SP,
+ node: hir::VisibilityKind::Inherited };
self.head("")?;
self.print_method_sig(ti.ident, sig, &ti.generics, &vis, &[], Some(body))?;
self.nbsp()?;
@@ -2267,7 +2270,8 @@ impl<'a> State<'a> {
},
name,
&generics,
- &Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited },
+ &Spanned { span: syntax_pos::DUMMY_SP,
+ node: hir::VisibilityKind::Inherited },
arg_names,
None)?;
self.end()
diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs
index 3170f9efafb..0c7baea85ad 100644
--- a/src/librustc/ich/impls_hir.rs
+++ b/src/librustc/ich/impls_hir.rs
@@ -710,20 +710,20 @@ impl_stable_hash_for!(enum ::syntax::ast::CrateSugar {
PubCrate,
});
-impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility_ {
+impl<'a> HashStable<StableHashingContext<'a>> for hir::VisibilityKind {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
- hir::VisibilityPublic |
- hir::VisibilityInherited => {
+ hir::VisibilityKind::Public |
+ hir::VisibilityKind::Inherited => {
// No fields to hash.
}
- hir::VisibilityCrate(sugar) => {
+ hir::VisibilityKind::Crate(sugar) => {
sugar.hash_stable(hcx, hasher);
}
- hir::VisibilityRestricted { ref path, id } => {
+ hir::VisibilityKind::Restricted { ref path, id } => {
hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
id.hash_stable(hcx, hasher);
});
@@ -733,7 +733,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for hir::Visibility_ {
}
}
-impl_stable_hash_for_spanned!(hir::Visibility_);
+impl_stable_hash_for_spanned!(hir::VisibilityKind);
impl<'a> HashStable<StableHashingContext<'a>> for hir::Defaultness {
fn hash_stable<W: StableHasherResult>(&self,
diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index aad60c0247d..54afd795fc0 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -269,15 +269,15 @@ impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> {
impl Visibility {
pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self {
match visibility.node {
- hir::VisibilityPublic => Visibility::Public,
- hir::VisibilityCrate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
- hir::VisibilityRestricted { ref path, .. } => match path.def {
+ hir::VisibilityKind::Public => Visibility::Public,
+ hir::VisibilityKind::Crate(_) => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
+ hir::VisibilityKind::Restricted { ref path, .. } => match path.def {
// If there is no resolution, `resolve` will have already reported an error, so
// assume that the visibility is public to avoid reporting more privacy errors.
Def::Err => Visibility::Public,
def => Visibility::Restricted(def.def_id()),
},
- hir::VisibilityInherited => {
+ hir::VisibilityKind::Inherited => {
Visibility::Restricted(tcx.hir.get_module_parent(id))
}
}
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index 96dcb458303..9ac4872622b 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -397,7 +397,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
hir::ItemUnion(..) => "a union",
hir::ItemTrait(.., ref trait_item_refs) => {
// Issue #11592, traits are always considered exported, even when private.
- if it.vis.node == hir::VisibilityInherited {
+ if it.vis.node == hir::VisibilityKind::Inherited {
self.private_traits.insert(it.id);
for trait_item_ref in trait_item_refs {
self.private_traits.insert(trait_item_ref.id.node_id);
@@ -414,7 +414,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
if let Some(node_id) = cx.tcx.hir.as_local_node_id(real_trait) {
match cx.tcx.hir.find(node_id) {
Some(hir_map::NodeItem(item)) => {
- if item.vis.node == hir::VisibilityInherited {
+ if item.vis.node == hir::VisibilityKind::Inherited {
for impl_item_ref in impl_item_refs {
self.private_traits.insert(impl_item_ref.id.node_id);
}
@@ -1179,15 +1179,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
let suggest_export = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
let suggestion = match vis.node {
- hir::VisibilityInherited => {
+ hir::VisibilityKind::Inherited => {
// inherited visibility is empty span at item start; need an extra space
Some("pub ".to_owned())
},
- hir::VisibilityRestricted { .. } |
- hir::VisibilityCrate(_) => {
+ hir::VisibilityKind::Restricted { .. } |
+ hir::VisibilityKind::Crate(_) => {
Some("pub".to_owned())
},
- hir::VisibilityPublic => {
+ hir::VisibilityKind::Public => {
err.help("try exporting the item with a `pub use` statement");
None
}
@@ -1399,7 +1399,7 @@ impl UnreachablePub {
vis: &hir::Visibility, span: Span, exportable: bool) {
let mut applicability = Applicability::MachineApplicable;
match vis.node {
- hir::VisibilityPublic if !cx.access_levels.is_reachable(id) => {
+ hir::VisibilityKind::Public if !cx.access_levels.is_reachable(id) => {
if span.ctxt().outer().expn_info().is_some() {
applicability = Applicability::MaybeIncorrect;
}
diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs
index 196c53970f6..3cfde7a8297 100644
--- a/src/librustc_metadata/encoder.rs
+++ b/src/librustc_metadata/encoder.rs
@@ -320,7 +320,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
fn encode_info_for_items(&mut self) -> Index {
let krate = self.tcx.hir.krate();
let mut index = IndexBuilder::new(self);
- let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityPublic };
+ let vis = Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityKind::Public };
index.record(DefId::local(CRATE_DEF_INDEX),
IsolatedEncoder::encode_info_for_mod,
FromId(CRATE_NODE_ID, (&krate.module, &krate.attrs, &vis)));
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
index dafb89be5fd..19f9abc60dd 100644
--- a/src/librustc_privacy/lib.rs
+++ b/src/librustc_privacy/lib.rs
@@ -1469,8 +1469,8 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'
}
if !vis.is_at_least(self.required_visibility, self.tcx) {
let vis_adj = match hir_vis.node {
- hir::VisibilityCrate(_) => "crate-visible",
- hir::VisibilityRestricted { .. } => "restricted",
+ hir::VisibilityKind::Crate(_) => "crate-visible",
+ hir::VisibilityKind::Restricted { .. } => "restricted",
_ => "private"
};
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
index 414247d22a8..447b5f1fe47 100644
--- a/src/librustc_save_analysis/lib.rs
+++ b/src/librustc_save_analysis/lib.rs
@@ -633,7 +633,7 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
..
}) |
Node::NodeVisibility(&Spanned {
- node: hir::VisibilityRestricted { ref path, .. }, .. }) => path.def,
+ node: hir::VisibilityKind::Restricted { ref path, .. }, .. }) => path.def,
Node::NodeExpr(&hir::Expr {
node: hir::ExprStruct(ref qpath, ..),
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 7394b6d580c..b8abb98edec 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -3226,10 +3226,10 @@ pub enum Visibility {
impl Clean<Option<Visibility>> for hir::Visibility {
fn clean(&self, cx: &DocContext) -> Option<Visibility> {
Some(match self.node {
- hir::VisibilityPublic => Visibility::Public,
- hir::VisibilityInherited => Visibility::Inherited,
- hir::VisibilityCrate(_) => Visibility::Crate,
- hir::VisibilityRestricted { ref path, .. } => {
+ hir::VisibilityKind::Public => Visibility::Public,
+ hir::VisibilityKind::Inherited => Visibility::Inherited,
+ hir::VisibilityKind::Crate(_) => Visibility::Crate,
+ hir::VisibilityKind::Restricted { ref path, .. } => {
let path = path.clean(cx);
let did = register_def(cx, path.def);
Visibility::Restricted(did, path)
diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs
index 76c11a4e7cd..6fd9ef234f4 100644
--- a/src/librustdoc/doctree.rs
+++ b/src/librustdoc/doctree.rs
@@ -54,7 +54,7 @@ impl Module {
Module {
name : name,
id: ast::CRATE_NODE_ID,
- vis: Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityInherited },
+ vis: Spanned { span: syntax_pos::DUMMY_SP, node: hir::VisibilityKind::Inherited },
stab: None,
depr: None,
where_outer: syntax_pos::DUMMY_SP,
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index 3a13f61327e..fdeba93990d 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -96,7 +96,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> {
self.module = self.visit_mod_contents(krate.span,
krate.attrs.clone(),
Spanned { span: syntax_pos::DUMMY_SP,
- node: hir::VisibilityPublic },
+ node: hir::VisibilityKind::Public },
ast::CRATE_NODE_ID,
&krate.module,
None);