Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a6a7dac

Browse files
borsbors
committedSep 23, 2017
Auto merge of #44633 - petrochenkov:priv2, r=nikomatsakis
Record semantic types for all syntactic types in bodies ... and use recorded types in type privacy checking (types are recorded after inference, so there are no `_`s left). Also use `hir_ty_to_ty` for types in signatures in type privacy checking. This could also be potentially useful for save-analysis and diagnostics. Fixes #42125 (comment) r? @eddyb
2 parents 85a5d3f + 419069d commit a6a7dac

14 files changed

+119
-144
lines changed
 

‎src/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/librustc/hir/lowering.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ impl<'a> LoweringContext<'a> {
683683
return self.lower_ty(ty);
684684
}
685685
TyKind::Path(ref qself, ref path) => {
686-
let id = self.lower_node_id(t.id).node_id;
686+
let id = self.lower_node_id(t.id);
687687
let qpath = self.lower_qpath(t.id, qself, path, ParamMode::Explicit);
688688
return self.ty_path(id, t.span, qpath);
689689
}
@@ -732,10 +732,12 @@ impl<'a> LoweringContext<'a> {
732732
TyKind::Mac(_) => panic!("TyMac should have been expanded by now."),
733733
};
734734

735+
let LoweredNodeId { node_id, hir_id } = self.lower_node_id(t.id);
735736
P(hir::Ty {
736-
id: self.lower_node_id(t.id).node_id,
737+
id: node_id,
737738
node: kind,
738739
span: t.span,
740+
hir_id,
739741
})
740742
}
741743

@@ -861,7 +863,7 @@ impl<'a> LoweringContext<'a> {
861863
// Otherwise, the base path is an implicit `Self` type path,
862864
// e.g. `Vec` in `Vec::new` or `<I as Iterator>::Item` in
863865
// `<I as Iterator>::Item::default`.
864-
let new_id = self.next_id().node_id;
866+
let new_id = self.next_id();
865867
self.ty_path(new_id, p.span, hir::QPath::Resolved(qself, path))
866868
};
867869

@@ -886,7 +888,7 @@ impl<'a> LoweringContext<'a> {
886888
}
887889

888890
// Wrap the associated extension in another type node.
889-
let new_id = self.next_id().node_id;
891+
let new_id = self.next_id();
890892
ty = self.ty_path(new_id, p.span, qpath);
891893
}
892894

@@ -994,7 +996,8 @@ impl<'a> LoweringContext<'a> {
994996
let &ParenthesizedParameterData { ref inputs, ref output, span } = data;
995997
let inputs = inputs.iter().map(|ty| self.lower_ty(ty)).collect();
996998
let mk_tup = |this: &mut Self, tys, span| {
997-
P(hir::Ty { node: hir::TyTup(tys), id: this.next_id().node_id, span })
999+
let LoweredNodeId { node_id, hir_id } = this.next_id();
1000+
P(hir::Ty { node: hir::TyTup(tys), id: node_id, hir_id, span })
9981001
};
9991002

10001003
hir::PathParameters {
@@ -2974,7 +2977,7 @@ impl<'a> LoweringContext<'a> {
29742977
self.expr_block(block, attrs)
29752978
}
29762979

2977-
fn ty_path(&mut self, id: NodeId, span: Span, qpath: hir::QPath) -> P<hir::Ty> {
2980+
fn ty_path(&mut self, id: LoweredNodeId, span: Span, qpath: hir::QPath) -> P<hir::Ty> {
29782981
let mut id = id;
29792982
let node = match qpath {
29802983
hir::QPath::Resolved(None, path) => {
@@ -2984,14 +2987,14 @@ impl<'a> LoweringContext<'a> {
29842987
bound_lifetimes: hir_vec![],
29852988
trait_ref: hir::TraitRef {
29862989
path: path.and_then(|path| path),
2987-
ref_id: id,
2990+
ref_id: id.node_id,
29882991
},
29892992
span,
29902993
};
29912994

29922995
// The original ID is taken by the `PolyTraitRef`,
29932996
// so the `Ty` itself needs a different one.
2994-
id = self.next_id().node_id;
2997+
id = self.next_id();
29952998

29962999
hir::TyTraitObject(hir_vec![principal], self.elided_lifetime(span))
29973000
} else {
@@ -3000,7 +3003,7 @@ impl<'a> LoweringContext<'a> {
30003003
}
30013004
_ => hir::TyPath(qpath)
30023005
};
3003-
P(hir::Ty { id, node, span })
3006+
P(hir::Ty { id: id.node_id, hir_id: id.hir_id, node, span })
30043007
}
30053008

30063009
fn elided_lifetime(&mut self, span: Span) -> hir::Lifetime {

‎src/librustc/hir/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,6 +1354,7 @@ pub struct Ty {
13541354
pub id: NodeId,
13551355
pub node: Ty_,
13561356
pub span: Span,
1357+
pub hir_id: HirId,
13571358
}
13581359

13591360
impl fmt::Debug for Ty {

‎src/librustc/ich/impls_hir.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Ty {
245245
hcx.while_hashing_hir_bodies(true, |hcx| {
246246
let hir::Ty {
247247
id: _,
248+
hir_id: _,
248249
ref node,
249250
ref span,
250251
} = *self;

‎src/librustc_privacy/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ crate-type = ["dylib"]
1010

1111
[dependencies]
1212
rustc = { path = "../librustc" }
13+
rustc_typeck = { path = "../librustc_typeck" }
1314
syntax = { path = "../libsyntax" }
1415
syntax_pos = { path = "../libsyntax_pos" }

‎src/librustc_privacy/lib.rs

Lines changed: 37 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#[macro_use] extern crate rustc;
1919
#[macro_use] extern crate syntax;
20+
extern crate rustc_typeck;
2021
extern crate syntax_pos;
2122

2223
use rustc::hir::{self, PatKind};
@@ -658,65 +659,6 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, 'tcx> {
658659
}
659660
false
660661
}
661-
662-
fn check_item(&mut self, item_id: ast::NodeId) -> &mut Self {
663-
self.current_item = self.tcx.hir.local_def_id(item_id);
664-
self.span = self.tcx.hir.span(item_id);
665-
self
666-
}
667-
668-
// Convenience methods for checking item interfaces
669-
fn ty(&mut self) -> &mut Self {
670-
self.tcx.type_of(self.current_item).visit_with(self);
671-
self
672-
}
673-
674-
fn generics(&mut self) -> &mut Self {
675-
for def in &self.tcx.generics_of(self.current_item).types {
676-
if def.has_default {
677-
self.tcx.type_of(def.def_id).visit_with(self);
678-
}
679-
}
680-
self
681-
}
682-
683-
fn predicates(&mut self) -> &mut Self {
684-
let predicates = self.tcx.predicates_of(self.current_item);
685-
for predicate in &predicates.predicates {
686-
predicate.visit_with(self);
687-
match predicate {
688-
&ty::Predicate::Trait(poly_predicate) => {
689-
self.check_trait_ref(poly_predicate.skip_binder().trait_ref);
690-
},
691-
&ty::Predicate::Projection(poly_predicate) => {
692-
let tcx = self.tcx;
693-
self.check_trait_ref(
694-
poly_predicate.skip_binder().projection_ty.trait_ref(tcx)
695-
);
696-
},
697-
_ => (),
698-
};
699-
}
700-
self
701-
}
702-
703-
fn impl_trait_ref(&mut self) -> &mut Self {
704-
if let Some(impl_trait_ref) = self.tcx.impl_trait_ref(self.current_item) {
705-
self.check_trait_ref(impl_trait_ref);
706-
}
707-
self.tcx.predicates_of(self.current_item).visit_with(self);
708-
self
709-
}
710-
711-
fn check_trait_ref(&mut self, trait_ref: ty::TraitRef<'tcx>) -> bool {
712-
if !self.item_is_accessible(trait_ref.def_id) {
713-
let msg = format!("trait `{}` is private", trait_ref);
714-
self.tcx.sess.span_err(self.span, &msg);
715-
return true;
716-
}
717-
718-
trait_ref.super_visit_with(self)
719-
}
720662
}
721663

722664
impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
@@ -733,6 +675,35 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
733675
self.tables = orig_tables;
734676
}
735677

678+
fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty) {
679+
self.span = hir_ty.span;
680+
if let Some(ty) = self.tables.node_id_to_type_opt(hir_ty.hir_id) {
681+
// Types in bodies.
682+
if ty.visit_with(self) {
683+
return;
684+
}
685+
} else {
686+
// Types in signatures.
687+
// FIXME: This is very ineffective. Ideally each HIR type should be converted
688+
// into a semantic type only once and the result should be cached somehow.
689+
if rustc_typeck::hir_ty_to_ty(self.tcx, hir_ty).visit_with(self) {
690+
return;
691+
}
692+
}
693+
694+
intravisit::walk_ty(self, hir_ty);
695+
}
696+
697+
fn visit_trait_ref(&mut self, trait_ref: &'tcx hir::TraitRef) {
698+
if !self.item_is_accessible(trait_ref.path.def.def_id()) {
699+
let msg = format!("trait `{:?}` is private", trait_ref.path);
700+
self.tcx.sess.span_err(self.span, &msg);
701+
return;
702+
}
703+
704+
intravisit::walk_trait_ref(self, trait_ref);
705+
}
706+
736707
// Check types of expressions
737708
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
738709
if self.check_expr_pat_type(expr.hir_id, expr.span) {
@@ -807,63 +778,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
807778
item.id,
808779
&mut self.tables,
809780
self.empty_tables);
810-
811-
match item.node {
812-
hir::ItemExternCrate(..) | hir::ItemMod(..) |
813-
hir::ItemUse(..) | hir::ItemGlobalAsm(..) => {}
814-
hir::ItemConst(..) | hir::ItemStatic(..) |
815-
hir::ItemTy(..) | hir::ItemFn(..) => {
816-
self.check_item(item.id).generics().predicates().ty();
817-
}
818-
hir::ItemTrait(.., ref trait_item_refs) => {
819-
self.check_item(item.id).generics().predicates();
820-
for trait_item_ref in trait_item_refs {
821-
let check = self.check_item(trait_item_ref.id.node_id);
822-
check.generics().predicates();
823-
if trait_item_ref.kind != hir::AssociatedItemKind::Type ||
824-
trait_item_ref.defaultness.has_value() {
825-
check.ty();
826-
}
827-
}
828-
}
829-
hir::ItemEnum(ref def, _) => {
830-
self.check_item(item.id).generics().predicates();
831-
for variant in &def.variants {
832-
for field in variant.node.data.fields() {
833-
self.check_item(field.id).ty();
834-
}
835-
}
836-
}
837-
hir::ItemForeignMod(ref foreign_mod) => {
838-
for foreign_item in &foreign_mod.items {
839-
self.check_item(foreign_item.id).generics().predicates().ty();
840-
}
841-
}
842-
hir::ItemStruct(ref struct_def, _) |
843-
hir::ItemUnion(ref struct_def, _) => {
844-
self.check_item(item.id).generics().predicates();
845-
for field in struct_def.fields() {
846-
self.check_item(field.id).ty();
847-
}
848-
}
849-
hir::ItemDefaultImpl(..) => {
850-
self.check_item(item.id).impl_trait_ref();
851-
}
852-
hir::ItemImpl(.., ref trait_ref, _, ref impl_item_refs) => {
853-
{
854-
let check = self.check_item(item.id);
855-
check.ty().generics().predicates();
856-
if trait_ref.is_some() {
857-
check.impl_trait_ref();
858-
}
859-
}
860-
for impl_item_ref in impl_item_refs {
861-
let impl_item = self.tcx.hir.impl_item(impl_item_ref.id);
862-
self.check_item(impl_item.id).generics().predicates().ty();
863-
}
864-
}
865-
}
866-
867781
self.current_item = self.tcx.hir.local_def_id(item.id);
868782
intravisit::walk_item(self, item);
869783
self.tables = orig_tables;
@@ -924,8 +838,13 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
924838
}
925839
}
926840
ty::TyProjection(ref proj) => {
927-
let tcx = self.tcx;
928-
if self.check_trait_ref(proj.trait_ref(tcx)) {
841+
let trait_ref = proj.trait_ref(self.tcx);
842+
if !self.item_is_accessible(trait_ref.def_id) {
843+
let msg = format!("trait `{}` is private", trait_ref);
844+
self.tcx.sess.span_err(self.span, &msg);
845+
return true;
846+
}
847+
if trait_ref.super_visit_with(self) {
929848
return true;
930849
}
931850
}

‎src/librustc_typeck/astconv.rs

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ pub trait AstConv<'gcx, 'tcx> {
7676
/// used to help suppress derived errors typeck might otherwise
7777
/// report.
7878
fn set_tainted_by_errors(&self);
79+
80+
fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, span: Span);
7981
}
8082

8183
struct ConvertedBinding<'tcx> {
@@ -975,6 +977,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
975977
}
976978
}
977979
Def::Err => {
980+
for segment in &path.segments {
981+
for ty in &segment.parameters.types {
982+
self.ast_ty_to_ty(ty);
983+
}
984+
for binding in &segment.parameters.bindings {
985+
self.ast_ty_to_ty(&binding.ty);
986+
}
987+
}
978988
self.set_tainted_by_errors();
979989
return self.tcx().types.err;
980990
}
@@ -1115,6 +1125,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
11151125
}
11161126
};
11171127

1128+
self.record_ty(ast_ty.hir_id, result_ty, ast_ty.span);
11181129
result_ty
11191130
}
11201131

@@ -1124,8 +1135,10 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
11241135
-> Ty<'tcx>
11251136
{
11261137
match ty.node {
1127-
hir::TyInfer if expected_ty.is_some() => expected_ty.unwrap(),
1128-
hir::TyInfer => self.ty_infer(ty.span),
1138+
hir::TyInfer if expected_ty.is_some() => {
1139+
self.record_ty(ty.hir_id, expected_ty.unwrap(), ty.span);
1140+
expected_ty.unwrap()
1141+
}
11291142
_ => self.ast_ty_to_ty(ty),
11301143
}
11311144
}
@@ -1214,19 +1227,22 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
12141227

12151228
let expected_ret_ty = expected_sig.as_ref().map(|e| e.output());
12161229

1217-
let is_infer = match decl.output {
1218-
hir::Return(ref output) if output.node == hir::TyInfer => true,
1219-
hir::DefaultReturn(..) => true,
1220-
_ => false
1221-
};
1222-
12231230
let output_ty = match decl.output {
1224-
_ if is_infer && expected_ret_ty.is_some() =>
1225-
expected_ret_ty.unwrap(),
1226-
_ if is_infer => self.ty_infer(decl.output.span()),
1227-
hir::Return(ref output) =>
1228-
self.ast_ty_to_ty(&output),
1229-
hir::DefaultReturn(..) => bug!(),
1231+
hir::Return(ref output) => {
1232+
if let (&hir::TyInfer, Some(expected_ret_ty)) = (&output.node, expected_ret_ty) {
1233+
self.record_ty(output.hir_id, expected_ret_ty, output.span);
1234+
expected_ret_ty
1235+
} else {
1236+
self.ast_ty_to_ty(&output)
1237+
}
1238+
}
1239+
hir::DefaultReturn(span) => {
1240+
if let Some(expected_ret_ty) = expected_ret_ty {
1241+
expected_ret_ty
1242+
} else {
1243+
self.ty_infer(span)
1244+
}
1245+
}
12301246
};
12311247

12321248
debug!("ty_of_closure: output_ty={:?}", output_ty);

‎src/librustc_typeck/check/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,10 @@ impl<'a, 'gcx, 'tcx> AstConv<'gcx, 'tcx> for FnCtxt<'a, 'gcx, 'tcx> {
16651665
fn set_tainted_by_errors(&self) {
16661666
self.infcx.set_tainted_by_errors()
16671667
}
1668+
1669+
fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, _span: Span) {
1670+
self.write_ty(hir_id, ty)
1671+
}
16681672
}
16691673

16701674
/// Controls whether the arguments are tupled. This is used for the call

‎src/librustc_typeck/check/writeback.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,13 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> {
207207
let var_ty = self.resolve(&var_ty, &l.span);
208208
self.write_ty_to_tables(l.hir_id, var_ty);
209209
}
210+
211+
fn visit_ty(&mut self, hir_ty: &'gcx hir::Ty) {
212+
intravisit::walk_ty(self, hir_ty);
213+
let ty = self.fcx.node_ty(hir_ty.hir_id);
214+
let ty = self.resolve(&ty, &hir_ty.span);
215+
self.write_ty_to_tables(hir_ty.hir_id, ty);
216+
}
210217
}
211218

212219
impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {

‎src/librustc_typeck/collect.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,10 @@ impl<'a, 'tcx> AstConv<'tcx, 'tcx> for ItemCtxt<'a, 'tcx> {
221221
fn set_tainted_by_errors(&self) {
222222
// no obvious place to track this, just let it go
223223
}
224+
225+
fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
226+
// no place to record types from signatures?
227+
}
224228
}
225229

226230
fn type_param_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

‎src/test/compile-fail/lint-stability-deprecated.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ mod cross_crate {
107107
struct S1<T: TraitWithAssociatedTypes>(T::TypeUnstable);
108108
struct S2<T: TraitWithAssociatedTypes>(T::TypeDeprecated);
109109
//~^ WARN use of deprecated item
110+
//~| WARN use of deprecated item
110111

111112
let _ = DeprecatedStruct { //~ WARN use of deprecated item
112113
i: 0 //~ WARN use of deprecated item

‎src/test/compile-fail/private-inferred-type.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,11 @@ mod adjust {
103103

104104
fn main() {
105105
let _: m::Alias; //~ ERROR type `m::Priv` is private
106-
let _: <m::Alias as m::TraitWithAssocTy>::AssocTy; // FIXME
106+
//~^ ERROR type `m::Priv` is private
107+
let _: <m::Alias as m::TraitWithAssocTy>::AssocTy; //~ ERROR type `m::Priv` is private
107108
m::Alias {}; //~ ERROR type `m::Priv` is private
108109
m::Pub { 0: m::Alias {} }; //~ ERROR type `m::Priv` is private
109-
m::Pub { 0: loop {} }; // FIXME
110+
m::Pub { 0: loop {} }; // OK, `m::Pub` is in value context, so it means Pub<_>, not Pub<Priv>
110111
m::Pub::static_method; //~ ERROR type `m::Priv` is private
111112
m::Pub::INHERENT_ASSOC_CONST; //~ ERROR type `m::Priv` is private
112113
m::Pub(0u8).method_with_substs::<m::Alias>(); //~ ERROR type `m::Priv` is private

‎src/test/compile-fail/private-type-in-interface.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ fn f_ext(_: ext::Alias) {} //~ ERROR type `ext::Priv` is private
3131
trait Tr1 {}
3232
impl m::Alias {} //~ ERROR type `m::Priv` is private
3333
impl Tr1 for ext::Alias {} //~ ERROR type `ext::Priv` is private
34-
//~^ ERROR type `ext::Priv` is private
3534
type A = <m::Alias as m::Trait>::X; //~ ERROR type `m::Priv` is private
3635

3736
trait Tr2<T> {}

‎src/test/compile-fail/type-path-err-node-types.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2017 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+
// Type arguments of unresolved types should have their types recorded
12+
13+
fn main() {
14+
let _: Nonexistent<u8, Assoc = u16>; //~ ERROR cannot find type `Nonexistent` in this scope
15+
16+
let _ = |a, b: _| -> _ { 0 };
17+
}

0 commit comments

Comments
 (0)
Please sign in to comment.