Skip to content

Commit 3c399f6

Browse files
borsbors
committed
auto merge of #10862 : alexcrichton/rust/issue-10857, r=huonw
This bug showed up because the visitor only visited the path of the implemented trait via walk_path (with no corresponding visit_path function). I have modified the visitor to use visit_path (which is now overridable), and the privacy visitor overrides this function and now properly checks for the privacy of all paths. Closes #10857
2 parents ad292ac + 9522a08 commit 3c399f6

File tree

7 files changed

+63
-35
lines changed

7 files changed

+63
-35
lines changed

‎src/librustc/middle/borrowck/check_loans.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ impl<'self> Visitor<()> for CheckLoanCtxt<'self> {
5858
b:ast::P<ast::Block>, s:Span, n:ast::NodeId, _:()) {
5959
check_loans_in_fn(self, fk, fd, b, s, n);
6060
}
61+
62+
// FIXME(#10894) should continue recursing
63+
fn visit_ty(&mut self, _t: &ast::Ty, _: ()) {}
6164
}
6265

6366
pub fn check_loans(bccx: &BorrowckCtxt,

‎src/librustc/middle/lint.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,9 @@ impl<'self> Visitor<()> for Context<'self> {
13641364
visit::walk_variant(cx, v, g, ());
13651365
})
13661366
}
1367+
1368+
// FIXME(#10894) should continue recursing
1369+
fn visit_ty(&mut self, _t: &ast::Ty, _: ()) {}
13671370
}
13681371

13691372
impl<'self> IdVisitingOperation for Context<'self> {

‎src/librustc/middle/moves.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ impl visit::Visitor<()> for VisitContext {
202202
fn visit_local(&mut self, l:@Local, _:()) {
203203
compute_modes_for_local(self, l);
204204
}
205+
// FIXME(#10894) should continue recursing
206+
fn visit_ty(&mut self, _t: &Ty, _: ()) {}
205207
}
206208

207209
pub fn compute_moves(tcx: ty::ctxt,

‎src/librustc/middle/privacy.rs

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ struct PrivacyVisitor<'self> {
312312
tcx: ty::ctxt,
313313
curitem: ast::NodeId,
314314
in_fn: bool,
315+
in_foreign: bool,
315316
method_map: &'self method_map,
316317
parents: HashMap<ast::NodeId, ast::NodeId>,
317318
external_exports: resolve::ExternalExports,
@@ -625,7 +626,9 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
625626
let t = ty::type_autoderef(self.tcx,
626627
ty::expr_ty(self.tcx, base));
627628
match ty::get(t).sty {
628-
ty::ty_struct(id, _) => self.check_field(expr.span, id, ident),
629+
ty::ty_struct(id, _) => {
630+
self.check_field(expr.span, id, ident);
631+
}
629632
_ => {}
630633
}
631634
}
@@ -649,9 +652,6 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
649652
_ => {}
650653
}
651654
}
652-
ast::ExprPath(ref path) => {
653-
self.check_path(expr.span, expr.id, path);
654-
}
655655
ast::ExprStruct(_, ref fields, _) => {
656656
match ty::get(ty::expr_ty(self.tcx, expr)).sty {
657657
ty::ty_struct(id, _) => {
@@ -697,25 +697,14 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
697697
visit::walk_expr(self, expr, ());
698698
}
699699

700-
fn visit_ty(&mut self, t: &ast::Ty, _: ()) {
701-
match t.node {
702-
ast::ty_path(ref path, _, id) => self.check_path(t.span, id, path),
703-
_ => {}
704-
}
705-
visit::walk_ty(self, t, ());
706-
}
707-
708700
fn visit_view_item(&mut self, a: &ast::view_item, _: ()) {
709701
match a.node {
710702
ast::view_item_extern_mod(..) => {}
711703
ast::view_item_use(ref uses) => {
712704
for vpath in uses.iter() {
713705
match vpath.node {
714-
ast::view_path_simple(_, ref path, id) |
715-
ast::view_path_glob(ref path, id) => {
716-
debug!("privacy - glob/simple {}", id);
717-
self.check_path(vpath.span, id, path);
718-
}
706+
ast::view_path_simple(..) |
707+
ast::view_path_glob(..) => {}
719708
ast::view_path_list(_, ref list, _) => {
720709
for pid in list.iter() {
721710
debug!("privacy - list {}", pid.node.id);
@@ -737,9 +726,16 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
737726
}
738727
}
739728
}
729+
visit::walk_view_item(self, a, ());
740730
}
741731

742732
fn visit_pat(&mut self, pattern: &ast::Pat, _: ()) {
733+
// Foreign functions do not have their patterns mapped in the def_map,
734+
// and there's nothing really relevant there anyway, so don't bother
735+
// checking privacy. If you can name the type then you can pass it to an
736+
// external C function anyway.
737+
if self.in_foreign { return }
738+
743739
match pattern.node {
744740
ast::PatStruct(_, ref fields, _) => {
745741
match ty::get(ty::pat_ty(self.tcx, pattern)).sty {
@@ -773,6 +769,17 @@ impl<'self> Visitor<()> for PrivacyVisitor<'self> {
773769

774770
visit::walk_pat(self, pattern, ());
775771
}
772+
773+
fn visit_foreign_item(&mut self, fi: @ast::foreign_item, _: ()) {
774+
self.in_foreign = true;
775+
visit::walk_foreign_item(self, fi, ());
776+
self.in_foreign = false;
777+
}
778+
779+
fn visit_path(&mut self, path: &ast::Path, id: ast::NodeId, _: ()) {
780+
self.check_path(path.span, id, path);
781+
visit::walk_path(self, path, ());
782+
}
776783
}
777784

778785
////////////////////////////////////////////////////////////////////////////////
@@ -999,6 +1006,7 @@ pub fn check_crate(tcx: ty::ctxt,
9991006
let mut visitor = PrivacyVisitor {
10001007
curitem: ast::DUMMY_NODE_ID,
10011008
in_fn: false,
1009+
in_foreign: false,
10021010
tcx: tcx,
10031011
parents: visitor.parents,
10041012
method_map: method_map,

‎src/librustc/middle/typeck/check/writeback.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,8 @@ impl Visitor<()> for WbCtxt {
325325
fn visit_block(&mut self, b:ast::P<ast::Block>, _:()) { visit_block(b, self); }
326326
fn visit_pat(&mut self, p:&ast::Pat, _:()) { visit_pat(p, self); }
327327
fn visit_local(&mut self, l:@ast::Local, _:()) { visit_local(l, self); }
328+
// FIXME(#10894) should continue recursing
329+
fn visit_ty(&mut self, _t: &ast::Ty, _:()) {}
328330
}
329331

330332
pub fn resolve_type_vars_in_expr(fcx: @mut FnCtxt, e: @ast::Expr) -> bool {

‎src/libsyntax/visit.rs

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ pub trait Visitor<E:Clone> {
8282
fn visit_decl(&mut self, d:@Decl, e:E) { walk_decl(self, d, e) }
8383
fn visit_expr(&mut self, ex:@Expr, e:E) { walk_expr(self, ex, e) }
8484
fn visit_expr_post(&mut self, _ex:@Expr, _e:E) { }
85-
fn visit_ty(&mut self, _t:&Ty, _e:E) { }
85+
fn visit_ty(&mut self, t:&Ty, e:E) { walk_ty(self, t, e) }
8686
fn visit_generics(&mut self, g:&Generics, e:E) { walk_generics(self, g, e) }
8787
fn visit_fn(&mut self, fk:&fn_kind, fd:&fn_decl, b:P<Block>, s:Span, n:NodeId, e:E) {
8888
walk_fn(self, fk, fd, b, s, n , e)
@@ -120,6 +120,9 @@ pub trait Visitor<E:Clone> {
120120
fn visit_mac(&mut self, macro:&mac, e:E) {
121121
walk_mac(self, macro, e)
122122
}
123+
fn visit_path(&mut self, path: &Path, _id: ast::NodeId, e: E) {
124+
walk_path(self, path, e)
125+
}
123126
}
124127

125128
pub fn walk_crate<E:Clone, V:Visitor<E>>(visitor: &mut V, crate: &Crate, env: E) {
@@ -143,21 +146,21 @@ pub fn walk_view_item<E:Clone, V:Visitor<E>>(visitor: &mut V, vi: &view_item, en
143146
}
144147
view_item_use(ref paths) => {
145148
for vp in paths.iter() {
146-
let path = match vp.node {
147-
view_path_simple(ident, ref path, _) => {
149+
match vp.node {
150+
view_path_simple(ident, ref path, id) => {
148151
visitor.visit_ident(vp.span, ident, env.clone());
149-
path
152+
visitor.visit_path(path, id, env.clone());
153+
}
154+
view_path_glob(ref path, id) => {
155+
visitor.visit_path(path, id, env.clone());
150156
}
151-
view_path_glob(ref path, _) => path,
152157
view_path_list(ref path, ref list, _) => {
153158
for id in list.iter() {
154159
visitor.visit_ident(id.span, id.node.name, env.clone())
155160
}
156-
path
161+
walk_path(visitor, path, env.clone());
157162
}
158-
};
159-
160-
walk_path(visitor, path, env.clone());
163+
}
161164
}
162165
}
163166
}
@@ -187,7 +190,7 @@ fn walk_explicit_self<E:Clone, V:Visitor<E>>(visitor: &mut V,
187190
fn walk_trait_ref<E:Clone, V:Visitor<E>>(visitor: &mut V,
188191
trait_ref: &ast::trait_ref,
189192
env: E) {
190-
walk_path(visitor, &trait_ref.path, env)
193+
visitor.visit_path(&trait_ref.path, trait_ref.ref_id, env)
191194
}
192195

193196
pub fn walk_item<E:Clone, V:Visitor<E>>(visitor: &mut V, item: &item, env: E) {
@@ -248,7 +251,9 @@ pub fn walk_item<E:Clone, V:Visitor<E>>(visitor: &mut V, item: &item, env: E) {
248251
item_trait(ref generics, ref trait_paths, ref methods) => {
249252
visitor.visit_generics(generics, env.clone());
250253
for trait_path in trait_paths.iter() {
251-
walk_path(visitor, &trait_path.path, env.clone())
254+
visitor.visit_path(&trait_path.path,
255+
trait_path.ref_id,
256+
env.clone())
252257
}
253258
for method in methods.iter() {
254259
visitor.visit_trait_method(method, env.clone())
@@ -331,8 +336,8 @@ pub fn walk_ty<E:Clone, V:Visitor<E>>(visitor: &mut V, typ: &Ty, env: E) {
331336
walk_lifetime_decls(visitor, &function_declaration.lifetimes,
332337
env.clone());
333338
}
334-
ty_path(ref path, ref bounds, _) => {
335-
walk_path(visitor, path, env.clone());
339+
ty_path(ref path, ref bounds, id) => {
340+
visitor.visit_path(path, id, env.clone());
336341
for bounds in bounds.iter() {
337342
walk_ty_param_bounds(visitor, bounds, env.clone())
338343
}
@@ -372,15 +377,15 @@ pub fn walk_path<E:Clone, V:Visitor<E>>(visitor: &mut V, path: &Path, env: E) {
372377
pub fn walk_pat<E:Clone, V:Visitor<E>>(visitor: &mut V, pattern: &Pat, env: E) {
373378
match pattern.node {
374379
PatEnum(ref path, ref children) => {
375-
walk_path(visitor, path, env.clone());
380+
visitor.visit_path(path, pattern.id, env.clone());
376381
for children in children.iter() {
377382
for child in children.iter() {
378383
visitor.visit_pat(*child, env.clone())
379384
}
380385
}
381386
}
382387
PatStruct(ref path, ref fields, _) => {
383-
walk_path(visitor, path, env.clone());
388+
visitor.visit_path(path, pattern.id, env.clone());
384389
for field in fields.iter() {
385390
visitor.visit_pat(field.pat, env.clone())
386391
}
@@ -396,7 +401,7 @@ pub fn walk_pat<E:Clone, V:Visitor<E>>(visitor: &mut V, pattern: &Pat, env: E) {
396401
visitor.visit_pat(subpattern, env)
397402
}
398403
PatIdent(_, ref path, ref optional_subpattern) => {
399-
walk_path(visitor, path, env.clone());
404+
visitor.visit_path(path, pattern.id, env.clone());
400405
match *optional_subpattern {
401406
None => {}
402407
Some(subpattern) => visitor.visit_pat(subpattern, env),
@@ -617,7 +622,7 @@ pub fn walk_expr<E:Clone, V:Visitor<E>>(visitor: &mut V, expression: @Expr, env:
617622
visitor.visit_expr(count, env.clone())
618623
}
619624
ExprStruct(ref path, ref fields, optional_base) => {
620-
walk_path(visitor, path, env.clone());
625+
visitor.visit_path(path, expression.id, env.clone());
621626
for field in fields.iter() {
622627
visitor.visit_expr(field.expr, env.clone())
623628
}
@@ -711,7 +716,9 @@ pub fn walk_expr<E:Clone, V:Visitor<E>>(visitor: &mut V, expression: @Expr, env:
711716
visitor.visit_expr(main_expression, env.clone());
712717
visitor.visit_expr(index_expression, env.clone())
713718
}
714-
ExprPath(ref path) => walk_path(visitor, path, env.clone()),
719+
ExprPath(ref path) => {
720+
visitor.visit_path(path, expression.id, env.clone())
721+
}
715722
ExprSelf | ExprBreak(_) | ExprAgain(_) => {}
716723
ExprRet(optional_expression) => {
717724
walk_expr_opt(visitor, optional_expression, env.clone())

‎src/test/compile-fail/privacy1.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ mod foo {
162162
bar::foo();
163163
bar::bar();
164164
}
165+
166+
impl ::bar::B for f32 { fn foo() -> f32 { 1.0 } }
167+
//~^ ERROR: trait `B` is private
165168
}
166169

167170
pub mod mytest {

0 commit comments

Comments
 (0)