Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate privacy into field and method selection #31938

Merged
merged 7 commits into from
Mar 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions src/librustc/front/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,14 +581,6 @@ impl<'ast> Map<'ast> {
}
}

pub fn get_foreign_vis(&self, id: NodeId) -> Visibility {
let vis = self.expect_foreign_item(id).vis; // read recorded by `expect_foreign_item`
match self.find(self.get_parent(id)) { // read recorded by `find`
Some(NodeItem(i)) => vis.inherit_from(i.vis),
_ => vis
}
}

pub fn expect_item(&self, id: NodeId) -> &'ast Item {
match self.find(id) { // read recorded by `find`
Some(NodeItem(item)) => item,
Expand Down
466 changes: 28 additions & 438 deletions src/librustc_privacy/lib.rs

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/librustc_typeck/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ pub enum MethodError<'tcx> {

// Using a `Fn`/`FnMut`/etc method on a raw closure type before we have inferred its kind.
ClosureAmbiguity(/* DefId of fn trait */ DefId),

// Found an applicable method, but it is not visible.
PrivateMatch(Def),
}

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

Expand Down
18 changes: 17 additions & 1 deletion src/librustc_typeck/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::suggest;
use check;
use check::{FnCtxt, UnresolvedTypeAction};
use middle::def_id::DefId;
use middle::def::Def;
use rustc::ty::subst;
use rustc::ty::subst::Subst;
use rustc::traits;
Expand Down Expand Up @@ -47,6 +48,9 @@ struct ProbeContext<'a, 'tcx:'a> {
/// used for error reporting
static_candidates: Vec<CandidateSource>,

/// Some(candidate) if there is a private candidate
private_candidate: Option<Def>,

/// Collects near misses when trait bounds for type parameters are unsatisfied and is only used
/// for error reporting
unsatisfied_predicates: Vec<TraitRef<'tcx>>
Expand Down Expand Up @@ -247,6 +251,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
steps: Rc::new(steps),
opt_simplified_steps: opt_simplified_steps,
static_candidates: Vec::new(),
private_candidate: None,
unsatisfied_predicates: Vec::new(),
}
}
Expand All @@ -256,6 +261,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
self.extension_candidates.clear();
self.impl_dups.clear();
self.static_candidates.clear();
self.private_candidate = None;
}

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

if item.vis() != hir::Public && !self.fcx.private_item_is_visible(item.def_id()) {
self.private_candidate = Some(item.def());
return
}

let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id);
let impl_ty = impl_ty.subst(self.tcx(), &impl_substs);

Expand Down Expand Up @@ -846,6 +857,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
}

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

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

if let Some(def) = private_candidate {
return Err(MethodError::PrivateMatch(def));
}

Err(MethodError::NoMatch(NoMatchData::new(static_candidates, unsatisfied_predicates,
out_of_scope_traits, self.mode)))
}
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_typeck/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
MethodError::NoMatch(NoMatchData { static_candidates: static_sources,
unsatisfied_predicates,
out_of_scope_traits,
mode }) => {
mode, .. }) => {
let cx = fcx.tcx();

let mut err = fcx.type_error_struct(
Expand Down Expand Up @@ -208,6 +208,11 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
};
fcx.sess().span_err(span, &msg);
}

MethodError::PrivateMatch(def) => {
let msg = format!("{} `{}` is private", def.kind_name(), item_name);
fcx.tcx().sess.span_err(span, &msg);
}
}

fn report_candidates(fcx: &FnCtxt,
Expand Down
114 changes: 67 additions & 47 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2938,25 +2938,26 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
base: &'tcx hir::Expr,
field: &Spanned<ast::Name>) {
check_expr_with_lvalue_pref(fcx, base, lvalue_pref);
let expr_t = structurally_resolved_type(fcx, expr.span,
fcx.expr_ty(base));
// FIXME(eddyb) #12808 Integrate privacy into this auto-deref loop.
let expr_t = structurally_resolved_type(fcx, expr.span, fcx.expr_ty(base));
let mut private_candidate = None;
let (_, autoderefs, field_ty) = autoderef(fcx,
expr.span,
expr_t,
|| Some(base),
UnresolvedTypeAction::Error,
lvalue_pref,
|base_t, _| {
match base_t.sty {
ty::TyStruct(base_def, substs) => {
debug!("struct named {:?}", base_t);
base_def.struct_variant()
.find_field_named(field.node)
.map(|f| fcx.field_ty(expr.span, f, substs))
if let ty::TyStruct(base_def, substs) = base_t.sty {
debug!("struct named {:?}", base_t);
if let Some(field) = base_def.struct_variant().find_field_named(field.node) {
let field_ty = fcx.field_ty(expr.span, field, substs);
if field.vis == hir::Public || fcx.private_item_is_visible(base_def.did) {
return Some(field_ty);
}
private_candidate = Some((base_def.did, field_ty));
}
_ => None
}
None
});
match field_ty {
Some(field_ty) => {
Expand All @@ -2967,12 +2968,14 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
None => {}
}

if field.node == special_idents::invalid.name {
if let Some((did, field_ty)) = private_candidate {
let struct_path = fcx.tcx().item_path_str(did);
let msg = format!("field `{}` of struct `{}` is private", field.node, struct_path);
fcx.tcx().sess.span_err(expr.span, &msg);
fcx.write_ty(expr.id, field_ty);
} else if field.node == special_idents::invalid.name {
fcx.write_error(expr.id);
return;
}

if method::exists(fcx, field.span, field.node, expr_t, expr.id) {
} else if method::exists(fcx, field.span, field.node, expr_t, expr.id) {
fcx.type_error_struct(field.span,
|actual| {
format!("attempted to take value of method `{}` on type \
Expand All @@ -2983,6 +2986,7 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
"maybe a `()` to call it is missing? \
If not, try an anonymous function")
.emit();
fcx.write_error(expr.id);
} else {
let mut err = fcx.type_error_struct(
expr.span,
Expand All @@ -2998,9 +3002,8 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
suggest_field_names(&mut err, def.struct_variant(), field, vec![]);
}
err.emit();
fcx.write_error(expr.id);
}

fcx.write_error(expr.id);
}

// displays hints about the closest matches in field names
Expand Down Expand Up @@ -3035,36 +3038,37 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
base: &'tcx hir::Expr,
idx: codemap::Spanned<usize>) {
check_expr_with_lvalue_pref(fcx, base, lvalue_pref);
let expr_t = structurally_resolved_type(fcx, expr.span,
fcx.expr_ty(base));
let expr_t = structurally_resolved_type(fcx, expr.span, fcx.expr_ty(base));
let mut private_candidate = None;
let mut tuple_like = false;
// FIXME(eddyb) #12808 Integrate privacy into this auto-deref loop.
let (_, autoderefs, field_ty) = autoderef(fcx,
expr.span,
expr_t,
|| Some(base),
UnresolvedTypeAction::Error,
lvalue_pref,
|base_t, _| {
match base_t.sty {
ty::TyStruct(base_def, substs) => {
tuple_like = base_def.struct_variant().is_tuple_struct();
if tuple_like {
debug!("tuple struct named {:?}", base_t);
base_def.struct_variant()
.fields
.get(idx.node)
.map(|f| fcx.field_ty(expr.span, f, substs))
} else {
None
}
}
let (base_def, substs) = match base_t.sty {
ty::TyStruct(base_def, substs) => (base_def, substs),
ty::TyTuple(ref v) => {
tuple_like = true;
if idx.node < v.len() { Some(v[idx.node]) } else { None }
return if idx.node < v.len() { Some(v[idx.node]) } else { None }
}
_ => None
_ => return None,
};

tuple_like = base_def.struct_variant().is_tuple_struct();
if !tuple_like { return None }

debug!("tuple struct named {:?}", base_t);
if let Some(field) = base_def.struct_variant().fields.get(idx.node) {
let field_ty = fcx.field_ty(expr.span, field, substs);
if field.vis == hir::Public || fcx.private_item_is_visible(base_def.did) {
return Some(field_ty);
}
private_candidate = Some((base_def.did, field_ty));
}
None
});
match field_ty {
Some(field_ty) => {
Expand All @@ -3074,6 +3078,15 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
}
None => {}
}

if let Some((did, field_ty)) = private_candidate {
let struct_path = fcx.tcx().item_path_str(did);
let msg = format!("field `{}` of struct `{}` is private", idx.node, struct_path);
fcx.tcx().sess.span_err(expr.span, &msg);
fcx.write_ty(expr.id, field_ty);
return;
}

fcx.type_error_message(
expr.span,
|actual| {
Expand Down Expand Up @@ -3744,23 +3757,30 @@ pub fn resolve_ty_and_def_ufcs<'a, 'b, 'tcx>(fcx: &FnCtxt<'b, 'tcx>,
&ty_segments[base_ty_end..]);
let item_segment = path.segments.last().unwrap();
let item_name = item_segment.identifier.name;
match method::resolve_ufcs(fcx, span, item_name, ty, node_id) {
Ok(def) => {
// Write back the new resolution.
fcx.ccx.tcx.def_map.borrow_mut()
.insert(node_id, def::PathResolution {
base_def: def,
depth: 0
});
Some((Some(ty), slice::ref_slice(item_segment), def))
}
let def = match method::resolve_ufcs(fcx, span, item_name, ty, node_id) {
Ok(def) => Some(def),
Err(error) => {
let def = match error {
method::MethodError::PrivateMatch(def) => Some(def),
_ => None,
};
if item_name != special_idents::invalid.name {
method::report_error(fcx, span, ty, item_name, None, error);
}
fcx.write_error(node_id);
None
def
}
};

if let Some(def) = def {
// Write back the new resolution.
fcx.ccx.tcx.def_map.borrow_mut().insert(node_id, def::PathResolution {
base_def: def,
depth: 0,
});
Some((Some(ty), slice::ref_slice(item_segment), def))
} else {
fcx.write_error(node_id);
None
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/compile-fail/issue-22684.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2016 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.

mod foo {
pub struct Foo;
impl Foo {
fn bar(&self) {}
}

pub trait Baz {
fn bar(&self) -> bool {}
}
impl Baz for Foo {}
}

fn main() {
use foo::Baz;

// Check that `bar` resolves to the trait method, not the inherent impl method.
let _: () = foo::Foo.bar(); //~ ERROR mismatched types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good

}
1 change: 1 addition & 0 deletions src/test/compile-fail/privacy1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ mod foo {
//~^ ERROR: method `bar` is private
::bar::baz::A.foo2(); //~ ERROR: module `baz` is private
::bar::baz::A.bar2(); //~ ERROR: module `baz` is private
//~^ ERROR: method `bar2` is private

let _: isize =
::bar::B::foo(); //~ ERROR: trait `B` is private
Expand Down
Loading