Skip to content

Commit 3399d19

Browse files
borsbors
committed
Auto merge of #31938 - jseyfried:autoderef_privacy, r=nikomatsakis
Integrate privacy into field and method selection This PR integrates privacy checking into field and method selection so that an inaccessible field/method can not stop an accessible field/method from being used (fixes #12808 and fixes #22684). r? @eddyb
2 parents 4583dc9 + 48c20b0 commit 3399d19

File tree

13 files changed

+236
-518
lines changed

13 files changed

+236
-518
lines changed

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

-8
Original file line numberDiff line numberDiff line change
@@ -581,14 +581,6 @@ impl<'ast> Map<'ast> {
581581
}
582582
}
583583

584-
pub fn get_foreign_vis(&self, id: NodeId) -> Visibility {
585-
let vis = self.expect_foreign_item(id).vis; // read recorded by `expect_foreign_item`
586-
match self.find(self.get_parent(id)) { // read recorded by `find`
587-
Some(NodeItem(i)) => vis.inherit_from(i.vis),
588-
_ => vis
589-
}
590-
}
591-
592584
pub fn expect_item(&self, id: NodeId) -> &'ast Item {
593585
match self.find(id) { // read recorded by `find`
594586
Some(NodeItem(item)) => item,

‎src/librustc_privacy/lib.rs

+28-438
Large diffs are not rendered by default.

‎src/librustc_typeck/check/method/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ pub enum MethodError<'tcx> {
4343

4444
// Using a `Fn`/`FnMut`/etc method on a raw closure type before we have inferred its kind.
4545
ClosureAmbiguity(/* DefId of fn trait */ DefId),
46+
47+
// Found an applicable method, but it is not visible.
48+
PrivateMatch(Def),
4649
}
4750

4851
// Contains a list of static methods that may apply, a list of unsatisfied trait predicates which
@@ -90,6 +93,7 @@ pub fn exists<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
9093
Err(NoMatch(..)) => false,
9194
Err(Ambiguity(..)) => true,
9295
Err(ClosureAmbiguity(..)) => true,
96+
Err(PrivateMatch(..)) => true,
9397
}
9498
}
9599

‎src/librustc_typeck/check/method/probe.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use super::suggest;
1616
use check;
1717
use check::{FnCtxt, UnresolvedTypeAction};
1818
use middle::def_id::DefId;
19+
use middle::def::Def;
1920
use rustc::ty::subst;
2021
use rustc::ty::subst::Subst;
2122
use rustc::traits;
@@ -47,6 +48,9 @@ struct ProbeContext<'a, 'tcx:'a> {
4748
/// used for error reporting
4849
static_candidates: Vec<CandidateSource>,
4950

51+
/// Some(candidate) if there is a private candidate
52+
private_candidate: Option<Def>,
53+
5054
/// Collects near misses when trait bounds for type parameters are unsatisfied and is only used
5155
/// for error reporting
5256
unsatisfied_predicates: Vec<TraitRef<'tcx>>
@@ -247,6 +251,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
247251
steps: Rc::new(steps),
248252
opt_simplified_steps: opt_simplified_steps,
249253
static_candidates: Vec::new(),
254+
private_candidate: None,
250255
unsatisfied_predicates: Vec::new(),
251256
}
252257
}
@@ -256,6 +261,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
256261
self.extension_candidates.clear();
257262
self.impl_dups.clear();
258263
self.static_candidates.clear();
264+
self.private_candidate = None;
259265
}
260266

261267
fn tcx(&self) -> &'a TyCtxt<'tcx> {
@@ -407,6 +413,11 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
407413
return self.record_static_candidate(ImplSource(impl_def_id));
408414
}
409415

416+
if item.vis() != hir::Public && !self.fcx.private_item_is_visible(item.def_id()) {
417+
self.private_candidate = Some(item.def());
418+
return
419+
}
420+
410421
let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id);
411422
let impl_ty = impl_ty.subst(self.tcx(), &impl_substs);
412423

@@ -846,6 +857,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
846857
}
847858

848859
let static_candidates = mem::replace(&mut self.static_candidates, vec![]);
860+
let private_candidate = mem::replace(&mut self.private_candidate, None);
849861
let unsatisfied_predicates = mem::replace(&mut self.unsatisfied_predicates, vec![]);
850862

851863
// things failed, so lets look at all traits, for diagnostic purposes now:
@@ -879,9 +891,13 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
879891
// this error only occurs when assembling candidates
880892
tcx.sess.span_bug(span, "encountered ClosureAmbiguity from pick_core");
881893
}
882-
None => vec![],
894+
_ => vec![],
883895
};
884896

897+
if let Some(def) = private_candidate {
898+
return Err(MethodError::PrivateMatch(def));
899+
}
900+
885901
Err(MethodError::NoMatch(NoMatchData::new(static_candidates, unsatisfied_predicates,
886902
out_of_scope_traits, self.mode)))
887903
}

‎src/librustc_typeck/check/method/suggest.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
9191
MethodError::NoMatch(NoMatchData { static_candidates: static_sources,
9292
unsatisfied_predicates,
9393
out_of_scope_traits,
94-
mode }) => {
94+
mode, .. }) => {
9595
let cx = fcx.tcx();
9696

9797
let mut err = fcx.type_error_struct(
@@ -208,6 +208,11 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
208208
};
209209
fcx.sess().span_err(span, &msg);
210210
}
211+
212+
MethodError::PrivateMatch(def) => {
213+
let msg = format!("{} `{}` is private", def.kind_name(), item_name);
214+
fcx.tcx().sess.span_err(span, &msg);
215+
}
211216
}
212217

213218
fn report_candidates(fcx: &FnCtxt,

‎src/librustc_typeck/check/mod.rs

+67-47
Original file line numberDiff line numberDiff line change
@@ -2939,25 +2939,26 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
29392939
base: &'tcx hir::Expr,
29402940
field: &Spanned<ast::Name>) {
29412941
check_expr_with_lvalue_pref(fcx, base, lvalue_pref);
2942-
let expr_t = structurally_resolved_type(fcx, expr.span,
2943-
fcx.expr_ty(base));
2944-
// FIXME(eddyb) #12808 Integrate privacy into this auto-deref loop.
2942+
let expr_t = structurally_resolved_type(fcx, expr.span, fcx.expr_ty(base));
2943+
let mut private_candidate = None;
29452944
let (_, autoderefs, field_ty) = autoderef(fcx,
29462945
expr.span,
29472946
expr_t,
29482947
|| Some(base),
29492948
UnresolvedTypeAction::Error,
29502949
lvalue_pref,
29512950
|base_t, _| {
2952-
match base_t.sty {
2953-
ty::TyStruct(base_def, substs) => {
2954-
debug!("struct named {:?}", base_t);
2955-
base_def.struct_variant()
2956-
.find_field_named(field.node)
2957-
.map(|f| fcx.field_ty(expr.span, f, substs))
2951+
if let ty::TyStruct(base_def, substs) = base_t.sty {
2952+
debug!("struct named {:?}", base_t);
2953+
if let Some(field) = base_def.struct_variant().find_field_named(field.node) {
2954+
let field_ty = fcx.field_ty(expr.span, field, substs);
2955+
if field.vis == hir::Public || fcx.private_item_is_visible(base_def.did) {
2956+
return Some(field_ty);
2957+
}
2958+
private_candidate = Some((base_def.did, field_ty));
29582959
}
2959-
_ => None
29602960
}
2961+
None
29612962
});
29622963
match field_ty {
29632964
Some(field_ty) => {
@@ -2968,12 +2969,14 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
29682969
None => {}
29692970
}
29702971

2971-
if field.node == special_idents::invalid.name {
2972+
if let Some((did, field_ty)) = private_candidate {
2973+
let struct_path = fcx.tcx().item_path_str(did);
2974+
let msg = format!("field `{}` of struct `{}` is private", field.node, struct_path);
2975+
fcx.tcx().sess.span_err(expr.span, &msg);
2976+
fcx.write_ty(expr.id, field_ty);
2977+
} else if field.node == special_idents::invalid.name {
29722978
fcx.write_error(expr.id);
2973-
return;
2974-
}
2975-
2976-
if method::exists(fcx, field.span, field.node, expr_t, expr.id) {
2979+
} else if method::exists(fcx, field.span, field.node, expr_t, expr.id) {
29772980
fcx.type_error_struct(field.span,
29782981
|actual| {
29792982
format!("attempted to take value of method `{}` on type \
@@ -2984,6 +2987,7 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
29842987
"maybe a `()` to call it is missing? \
29852988
If not, try an anonymous function")
29862989
.emit();
2990+
fcx.write_error(expr.id);
29872991
} else {
29882992
let mut err = fcx.type_error_struct(
29892993
expr.span,
@@ -2999,9 +3003,8 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
29993003
suggest_field_names(&mut err, def.struct_variant(), field, vec![]);
30003004
}
30013005
err.emit();
3006+
fcx.write_error(expr.id);
30023007
}
3003-
3004-
fcx.write_error(expr.id);
30053008
}
30063009

30073010
// displays hints about the closest matches in field names
@@ -3036,36 +3039,37 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
30363039
base: &'tcx hir::Expr,
30373040
idx: codemap::Spanned<usize>) {
30383041
check_expr_with_lvalue_pref(fcx, base, lvalue_pref);
3039-
let expr_t = structurally_resolved_type(fcx, expr.span,
3040-
fcx.expr_ty(base));
3042+
let expr_t = structurally_resolved_type(fcx, expr.span, fcx.expr_ty(base));
3043+
let mut private_candidate = None;
30413044
let mut tuple_like = false;
3042-
// FIXME(eddyb) #12808 Integrate privacy into this auto-deref loop.
30433045
let (_, autoderefs, field_ty) = autoderef(fcx,
30443046
expr.span,
30453047
expr_t,
30463048
|| Some(base),
30473049
UnresolvedTypeAction::Error,
30483050
lvalue_pref,
30493051
|base_t, _| {
3050-
match base_t.sty {
3051-
ty::TyStruct(base_def, substs) => {
3052-
tuple_like = base_def.struct_variant().is_tuple_struct();
3053-
if tuple_like {
3054-
debug!("tuple struct named {:?}", base_t);
3055-
base_def.struct_variant()
3056-
.fields
3057-
.get(idx.node)
3058-
.map(|f| fcx.field_ty(expr.span, f, substs))
3059-
} else {
3060-
None
3061-
}
3062-
}
3052+
let (base_def, substs) = match base_t.sty {
3053+
ty::TyStruct(base_def, substs) => (base_def, substs),
30633054
ty::TyTuple(ref v) => {
30643055
tuple_like = true;
3065-
if idx.node < v.len() { Some(v[idx.node]) } else { None }
3056+
return if idx.node < v.len() { Some(v[idx.node]) } else { None }
30663057
}
3067-
_ => None
3058+
_ => return None,
3059+
};
3060+
3061+
tuple_like = base_def.struct_variant().is_tuple_struct();
3062+
if !tuple_like { return None }
3063+
3064+
debug!("tuple struct named {:?}", base_t);
3065+
if let Some(field) = base_def.struct_variant().fields.get(idx.node) {
3066+
let field_ty = fcx.field_ty(expr.span, field, substs);
3067+
if field.vis == hir::Public || fcx.private_item_is_visible(base_def.did) {
3068+
return Some(field_ty);
3069+
}
3070+
private_candidate = Some((base_def.did, field_ty));
30683071
}
3072+
None
30693073
});
30703074
match field_ty {
30713075
Some(field_ty) => {
@@ -3075,6 +3079,15 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
30753079
}
30763080
None => {}
30773081
}
3082+
3083+
if let Some((did, field_ty)) = private_candidate {
3084+
let struct_path = fcx.tcx().item_path_str(did);
3085+
let msg = format!("field `{}` of struct `{}` is private", idx.node, struct_path);
3086+
fcx.tcx().sess.span_err(expr.span, &msg);
3087+
fcx.write_ty(expr.id, field_ty);
3088+
return;
3089+
}
3090+
30783091
fcx.type_error_message(
30793092
expr.span,
30803093
|actual| {
@@ -3745,23 +3758,30 @@ pub fn resolve_ty_and_def_ufcs<'a, 'b, 'tcx>(fcx: &FnCtxt<'b, 'tcx>,
37453758
&ty_segments[base_ty_end..]);
37463759
let item_segment = path.segments.last().unwrap();
37473760
let item_name = item_segment.identifier.name;
3748-
match method::resolve_ufcs(fcx, span, item_name, ty, node_id) {
3749-
Ok(def) => {
3750-
// Write back the new resolution.
3751-
fcx.ccx.tcx.def_map.borrow_mut()
3752-
.insert(node_id, def::PathResolution {
3753-
base_def: def,
3754-
depth: 0
3755-
});
3756-
Some((Some(ty), slice::ref_slice(item_segment), def))
3757-
}
3761+
let def = match method::resolve_ufcs(fcx, span, item_name, ty, node_id) {
3762+
Ok(def) => Some(def),
37583763
Err(error) => {
3764+
let def = match error {
3765+
method::MethodError::PrivateMatch(def) => Some(def),
3766+
_ => None,
3767+
};
37593768
if item_name != special_idents::invalid.name {
37603769
method::report_error(fcx, span, ty, item_name, None, error);
37613770
}
3762-
fcx.write_error(node_id);
3763-
None
3771+
def
37643772
}
3773+
};
3774+
3775+
if let Some(def) = def {
3776+
// Write back the new resolution.
3777+
fcx.ccx.tcx.def_map.borrow_mut().insert(node_id, def::PathResolution {
3778+
base_def: def,
3779+
depth: 0,
3780+
});
3781+
Some((Some(ty), slice::ref_slice(item_segment), def))
3782+
} else {
3783+
fcx.write_error(node_id);
3784+
None
37653785
}
37663786
}
37673787
}

‎src/test/compile-fail/issue-22684.rs

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
mod foo {
12+
pub struct Foo;
13+
impl Foo {
14+
fn bar(&self) {}
15+
}
16+
17+
pub trait Baz {
18+
fn bar(&self) -> bool {}
19+
}
20+
impl Baz for Foo {}
21+
}
22+
23+
fn main() {
24+
use foo::Baz;
25+
26+
// Check that `bar` resolves to the trait method, not the inherent impl method.
27+
let _: () = foo::Foo.bar(); //~ ERROR mismatched types
28+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ mod foo {
102102
//~^ ERROR: method `bar` is private
103103
::bar::baz::A.foo2(); //~ ERROR: module `baz` is private
104104
::bar::baz::A.bar2(); //~ ERROR: module `baz` is private
105+
//~^ ERROR: method `bar2` is private
105106

106107
let _: isize =
107108
::bar::B::foo(); //~ ERROR: trait `B` is private

0 commit comments

Comments
 (0)