Skip to content

Commit 82e0303

Browse files
petrochenkovpetrochenkov
committedApr 25, 2017
Check privacy of trait items in all contexts
·
1.86.01.19.0
1 parent 2b4c911 commit 82e0303

File tree

9 files changed

+162
-176
lines changed

9 files changed

+162
-176
lines changed
 

Diff for: ‎src/librustc/ty/mod.rs

+11-14
Original file line numberDiff line numberDiff line change
@@ -2156,6 +2156,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
21562156

21572157
fn associated_item_from_trait_item_ref(self,
21582158
parent_def_id: DefId,
2159+
parent_vis: &hir::Visibility,
21592160
trait_item_ref: &hir::TraitItemRef)
21602161
-> AssociatedItem {
21612162
let def_id = self.hir.local_def_id(trait_item_ref.id.node_id);
@@ -2170,7 +2171,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
21702171
AssociatedItem {
21712172
name: trait_item_ref.name,
21722173
kind: kind,
2173-
vis: Visibility::from_hir(&hir::Inherited, trait_item_ref.id.node_id, self),
2174+
// Visibility of trait items is inherited from their traits.
2175+
vis: Visibility::from_hir(parent_vis, trait_item_ref.id.node_id, self),
21742176
defaultness: trait_item_ref.defaultness,
21752177
def_id: def_id,
21762178
container: TraitContainer(parent_def_id),
@@ -2180,7 +2182,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
21802182

21812183
fn associated_item_from_impl_item_ref(self,
21822184
parent_def_id: DefId,
2183-
from_trait_impl: bool,
21842185
impl_item_ref: &hir::ImplItemRef)
21852186
-> AssociatedItem {
21862187
let def_id = self.hir.local_def_id(impl_item_ref.id.node_id);
@@ -2192,14 +2193,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
21922193
hir::AssociatedItemKind::Type => (ty::AssociatedKind::Type, false),
21932194
};
21942195

2195-
// Trait impl items are always public.
2196-
let public = hir::Public;
2197-
let vis = if from_trait_impl { &public } else { &impl_item_ref.vis };
2198-
21992196
ty::AssociatedItem {
22002197
name: impl_item_ref.name,
22012198
kind: kind,
2202-
vis: ty::Visibility::from_hir(vis, impl_item_ref.id.node_id, self),
2199+
// Visibility of trait impl items doesn't matter.
2200+
vis: ty::Visibility::from_hir(&impl_item_ref.vis, impl_item_ref.id.node_id, self),
22032201
defaultness: impl_item_ref.defaultness,
22042202
def_id: def_id,
22052203
container: ImplContainer(parent_def_id),
@@ -2639,21 +2637,20 @@ fn associated_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
26392637
let parent_def_id = tcx.hir.local_def_id(parent_id);
26402638
let parent_item = tcx.hir.expect_item(parent_id);
26412639
match parent_item.node {
2642-
hir::ItemImpl(.., ref impl_trait_ref, _, ref impl_item_refs) => {
2640+
hir::ItemImpl(.., ref impl_item_refs) => {
26432641
if let Some(impl_item_ref) = impl_item_refs.iter().find(|i| i.id.node_id == id) {
2644-
let assoc_item =
2645-
tcx.associated_item_from_impl_item_ref(parent_def_id,
2646-
impl_trait_ref.is_some(),
2647-
impl_item_ref);
2642+
let assoc_item = tcx.associated_item_from_impl_item_ref(parent_def_id,
2643+
impl_item_ref);
26482644
debug_assert_eq!(assoc_item.def_id, def_id);
26492645
return assoc_item;
26502646
}
26512647
}
26522648

26532649
hir::ItemTrait(.., ref trait_item_refs) => {
26542650
if let Some(trait_item_ref) = trait_item_refs.iter().find(|i| i.id.node_id == id) {
2655-
let assoc_item =
2656-
tcx.associated_item_from_trait_item_ref(parent_def_id, trait_item_ref);
2651+
let assoc_item = tcx.associated_item_from_trait_item_ref(parent_def_id,
2652+
&parent_item.vis,
2653+
trait_item_ref);
26572654
debug_assert_eq!(assoc_item.def_id, def_id);
26582655
return assoc_item;
26592656
}

Diff for: ‎src/librustc_privacy/lib.rs

-27
Original file line numberDiff line numberDiff line change
@@ -427,14 +427,6 @@ struct PrivacyVisitor<'a, 'tcx: 'a> {
427427
}
428428

429429
impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
430-
fn item_is_accessible(&self, did: DefId) -> bool {
431-
match self.tcx.hir.as_local_node_id(did) {
432-
Some(node_id) =>
433-
ty::Visibility::from_hir(&self.tcx.hir.expect_item(node_id).vis, node_id, self.tcx),
434-
None => self.tcx.sess.cstore.visibility(did),
435-
}.is_accessible_from(self.curitem, self.tcx)
436-
}
437-
438430
// Checks that a field is in scope.
439431
fn check_field(&mut self, span: Span, def: &'tcx ty::AdtDef, field: &'tcx ty::FieldDef) {
440432
if !def.is_enum() && !field.vis.is_accessible_from(self.curitem, self.tcx) {
@@ -444,20 +436,6 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
444436
.emit();
445437
}
446438
}
447-
448-
// Checks that a method is in scope.
449-
fn check_method(&mut self, span: Span, method_def_id: DefId) {
450-
match self.tcx.associated_item(method_def_id).container {
451-
// Trait methods are always all public. The only controlling factor
452-
// is whether the trait itself is accessible or not.
453-
ty::TraitContainer(trait_def_id) if !self.item_is_accessible(trait_def_id) => {
454-
let msg = format!("source trait `{}` is private",
455-
self.tcx.item_path_str(trait_def_id));
456-
self.tcx.sess.span_err(span, &msg);
457-
}
458-
_ => {}
459-
}
460-
}
461439
}
462440

463441
impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> {
@@ -483,11 +461,6 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> {
483461

484462
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
485463
match expr.node {
486-
hir::ExprMethodCall(..) => {
487-
let method_call = ty::MethodCall::expr(expr.id);
488-
let method = self.tables.method_map[&method_call];
489-
self.check_method(expr.span, method.def_id);
490-
}
491464
hir::ExprStruct(ref qpath, ref expr_fields, _) => {
492465
let def = self.tables.qpath_def(qpath, expr.id);
493466
let adt = self.tables.expr_ty(expr).ty_adt_def().unwrap();

Diff for: ‎src/librustc_typeck/astconv.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -903,10 +903,16 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
903903
let ty = self.projected_ty_from_poly_trait_ref(span, bound, assoc_name);
904904
let ty = self.normalize_ty(span, ty);
905905

906-
let item = tcx.associated_items(trait_did).find(|i| i.name == assoc_name);
907-
let def_id = item.expect("missing associated type").def_id;
908-
tcx.check_stability(def_id, ref_id, span);
909-
(ty, Def::AssociatedTy(def_id))
906+
let item = tcx.associated_items(trait_did).find(|i| i.name == assoc_name)
907+
.expect("missing associated type");
908+
let def = Def::AssociatedTy(item.def_id);
909+
if !tcx.vis_is_accessible_from(item.vis, ref_id) {
910+
let msg = format!("{} `{}` is private", def.kind_name(), assoc_name);
911+
tcx.sess.span_err(span, &msg);
912+
}
913+
tcx.check_stability(item.def_id, ref_id, span);
914+
915+
(ty, def)
910916
}
911917

912918
fn qpath_to_ty(&self,

Diff for: ‎src/librustc_typeck/check/method/mod.rs

-7
Original file line numberDiff line numberDiff line change
@@ -349,15 +349,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
349349
}
350350

351351
let def = pick.item.def();
352-
353352
self.tcx.check_stability(def.def_id(), expr_id, span);
354353

355-
if let probe::InherentImplPick = pick.kind {
356-
if !self.tcx.vis_is_accessible_from(pick.item.vis, self.body_id) {
357-
let msg = format!("{} `{}` is private", def.kind_name(), method_name);
358-
self.tcx.sess.span_err(span, &msg);
359-
}
360-
}
361354
Ok(def)
362355
}
363356

Diff for: ‎src/librustc_typeck/check/method/probe.rs

+30-48
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,24 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
369369
///////////////////////////////////////////////////////////////////////////
370370
// CANDIDATE ASSEMBLY
371371

372+
fn push_inherent_candidate(&mut self, xform_self_ty: Ty<'tcx>, item: ty::AssociatedItem,
373+
kind: CandidateKind<'tcx>, import_id: Option<ast::NodeId>) {
374+
if self.tcx.vis_is_accessible_from(item.vis, self.body_id) {
375+
self.inherent_candidates.push(Candidate { xform_self_ty, item, kind, import_id });
376+
} else if self.private_candidate.is_none() {
377+
self.private_candidate = Some(item.def());
378+
}
379+
}
380+
381+
fn push_extension_candidate(&mut self, xform_self_ty: Ty<'tcx>, item: ty::AssociatedItem,
382+
kind: CandidateKind<'tcx>, import_id: Option<ast::NodeId>) {
383+
if self.tcx.vis_is_accessible_from(item.vis, self.body_id) {
384+
self.extension_candidates.push(Candidate { xform_self_ty, item, kind, import_id });
385+
} else if self.private_candidate.is_none() {
386+
self.private_candidate = Some(item.def());
387+
}
388+
}
389+
372390
fn assemble_inherent_candidates(&mut self) {
373391
let steps = self.steps.clone();
374392
for step in steps.iter() {
@@ -499,11 +517,6 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
499517
continue
500518
}
501519

502-
if !self.tcx.vis_is_accessible_from(item.vis, self.body_id) {
503-
self.private_candidate = Some(item.def());
504-
continue
505-
}
506-
507520
let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id);
508521
let impl_ty = impl_ty.subst(self.tcx, impl_substs);
509522

@@ -519,12 +532,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
519532
debug!("assemble_inherent_impl_probe: xform_self_ty = {:?}",
520533
xform_self_ty);
521534

522-
self.inherent_candidates.push(Candidate {
523-
xform_self_ty: xform_self_ty,
524-
item: item,
525-
kind: InherentImplCandidate(impl_substs, obligations),
526-
import_id: None,
527-
});
535+
self.push_inherent_candidate(xform_self_ty, item,
536+
InherentImplCandidate(impl_substs, obligations), None);
528537
}
529538
}
530539

@@ -548,12 +557,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
548557
let xform_self_ty =
549558
this.xform_self_ty(&item, new_trait_ref.self_ty(), new_trait_ref.substs);
550559

551-
this.inherent_candidates.push(Candidate {
552-
xform_self_ty: xform_self_ty,
553-
item: item,
554-
kind: ObjectCandidate,
555-
import_id: None,
556-
});
560+
this.push_inherent_candidate(xform_self_ty, item, ObjectCandidate, None);
557561
});
558562
}
559563

@@ -599,12 +603,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
599603
// `WhereClausePick`.
600604
assert!(!trait_ref.substs.needs_infer());
601605

602-
this.inherent_candidates.push(Candidate {
603-
xform_self_ty: xform_self_ty,
604-
item: item,
605-
kind: WhereClauseCandidate(poly_trait_ref),
606-
import_id: None,
607-
});
606+
this.push_inherent_candidate(xform_self_ty, item,
607+
WhereClauseCandidate(poly_trait_ref), None);
608608
});
609609
}
610610

@@ -743,12 +743,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
743743

744744
debug!("xform_self_ty={:?}", xform_self_ty);
745745

746-
self.extension_candidates.push(Candidate {
747-
xform_self_ty: xform_self_ty,
748-
item: item.clone(),
749-
kind: ExtensionImplCandidate(impl_def_id, impl_substs, obligations),
750-
import_id: import_id,
751-
});
746+
self.push_extension_candidate(xform_self_ty, item,
747+
ExtensionImplCandidate(impl_def_id, impl_substs, obligations), import_id);
752748
});
753749
}
754750

@@ -833,12 +829,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
833829
});
834830

835831
let xform_self_ty = self.xform_self_ty(&item, step.self_ty, substs);
836-
self.inherent_candidates.push(Candidate {
837-
xform_self_ty: xform_self_ty,
838-
item: item.clone(),
839-
kind: TraitCandidate,
840-
import_id: import_id,
841-
});
832+
self.push_inherent_candidate(xform_self_ty, item, TraitCandidate, import_id);
842833
}
843834

844835
Ok(())
@@ -854,7 +845,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
854845
trait_def_id,
855846
item);
856847

857-
for step in self.steps.iter() {
848+
for step in Rc::clone(&self.steps).iter() {
858849
debug!("assemble_projection_candidates: step={:?}", step);
859850

860851
let (def_id, substs) = match step.self_ty.sty {
@@ -889,12 +880,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
889880
bound,
890881
xform_self_ty);
891882

892-
self.extension_candidates.push(Candidate {
893-
xform_self_ty: xform_self_ty,
894-
item: item.clone(),
895-
kind: TraitCandidate,
896-
import_id: import_id,
897-
});
883+
self.push_extension_candidate(xform_self_ty, item, TraitCandidate, import_id);
898884
}
899885
}
900886
}
@@ -918,12 +904,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
918904
bound,
919905
xform_self_ty);
920906

921-
self.extension_candidates.push(Candidate {
922-
xform_self_ty: xform_self_ty,
923-
item: item.clone(),
924-
kind: WhereClauseCandidate(poly_bound),
925-
import_id: import_id,
926-
});
907+
self.push_extension_candidate(xform_self_ty, item,
908+
WhereClauseCandidate(poly_bound), import_id);
927909
}
928910
}
929911

Diff for: ‎src/test/compile-fail/issue-28514.rs

-47
This file was deleted.

Diff for: ‎src/test/compile-fail/no-method-suggested-traits.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct Foo;
1616
enum Bar { X }
1717

1818
mod foo {
19-
trait Bar {
19+
pub trait Bar {
2020
fn method(&self) {}
2121

2222
fn method2(&self) {}

Diff for: ‎src/test/compile-fail/trait-item-privacy.rs

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(associated_consts)]
12+
#![feature(associated_type_defaults)]
13+
14+
struct S;
15+
16+
mod method {
17+
trait A {
18+
fn a(&self) { }
19+
const A: u8 = 0;
20+
}
21+
22+
pub trait B {
23+
fn b(&self) { }
24+
const B: u8 = 0;
25+
}
26+
27+
pub trait C: A + B {
28+
fn c(&self) { }
29+
const C: u8 = 0;
30+
}
31+
32+
impl A for ::S {}
33+
impl B for ::S {}
34+
impl C for ::S {}
35+
36+
}
37+
38+
mod assoc_ty {
39+
trait A {
40+
type A = u8;
41+
}
42+
43+
pub trait B {
44+
type B = u8;
45+
}
46+
47+
pub trait C: A + B {
48+
type C = u8;
49+
}
50+
51+
impl A for ::S {}
52+
impl B for ::S {}
53+
impl C for ::S {}
54+
55+
}
56+
57+
fn check_assoc_ty<T: assoc_ty::C>() {
58+
// A is private
59+
// B is pub, not in scope
60+
// C : A + B is pub, in scope
61+
use assoc_ty::C;
62+
63+
// Associated types
64+
// A, B, C are resolved as trait items, their traits need to be in scope, not implemented yet
65+
let _: S::A; //~ ERROR ambiguous associated type
66+
let _: S::B; //~ ERROR ambiguous associated type
67+
let _: S::C; //~ ERROR ambiguous associated type
68+
// A, B, C are resolved as inherent items, their traits don't need to be in scope
69+
let _: T::A; //~ ERROR associated type `A` is private
70+
let _: T::B; // OK
71+
let _: T::C; // OK
72+
}
73+
74+
fn main() {
75+
// A is private
76+
// B is pub, not in scope
77+
// C : A + B is pub, in scope
78+
use method::C;
79+
80+
// Methods, method call
81+
// a, b, c are resolved as trait items, their traits need to be in scope
82+
S.a(); //~ ERROR no method named `a` found for type `S` in the current scope
83+
S.b(); //~ ERROR no method named `b` found for type `S` in the current scope
84+
S.c(); // OK
85+
// a, b, c are resolved as inherent items, their traits don't need to be in scope
86+
let c = &S as &C;
87+
c.a(); //~ ERROR method `a` is private
88+
c.b(); // OK
89+
c.c(); // OK
90+
91+
// Methods, UFCS
92+
// a, b, c are resolved as trait items, their traits need to be in scope
93+
S::a(&S); //~ ERROR no associated item named `a` found for type `S` in the current scope
94+
S::b(&S); //~ ERROR no associated item named `b` found for type `S` in the current scope
95+
S::c(&S); // OK
96+
// a, b, c are resolved as inherent items, their traits don't need to be in scope
97+
C::a(&S); //~ ERROR method `a` is private
98+
C::b(&S); // OK
99+
C::c(&S); // OK
100+
101+
// Associated constants
102+
// A, B, C are resolved as trait items, their traits need to be in scope
103+
S::A; //~ ERROR no associated item named `A` found for type `S` in the current scope
104+
S::B; //~ ERROR no associated item named `B` found for type `S` in the current scope
105+
S::C; // OK
106+
// A, B, C are resolved as inherent items, their traits don't need to be in scope
107+
C::A; //~ ERROR associated constant `A` is private
108+
C::B; // OK
109+
C::C; // OK
110+
}

Diff for: ‎src/test/compile-fail/trait-not-accessible.rs

-28
This file was deleted.

0 commit comments

Comments
 (0)
Please sign in to comment.