Skip to content

Commit 9d4422e

Browse files
ManishearthManishearth
committed
Rollup merge of #31919 - petrochenkov:blockpub, r=nikomatsakis
Closes #31776 r? @nikomatsakis
2 parents 84d8fec + b6f441d commit 9d4422e

File tree

4 files changed

+97
-129
lines changed

4 files changed

+97
-129
lines changed

‎src/librustc_privacy/lib.rs

Lines changed: 15 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,43 +1129,19 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
11291129

11301130
struct SanePrivacyVisitor<'a, 'tcx: 'a> {
11311131
tcx: &'a ty::ctxt<'tcx>,
1132-
in_block: bool,
11331132
}
11341133

11351134
impl<'a, 'tcx, 'v> Visitor<'v> for SanePrivacyVisitor<'a, 'tcx> {
1136-
/// We want to visit items in the context of their containing
1137-
/// module and so forth, so supply a crate for doing a deep walk.
1138-
fn visit_nested_item(&mut self, item: hir::ItemId) {
1139-
self.visit_item(self.tcx.map.expect_item(item.id))
1140-
}
1141-
11421135
fn visit_item(&mut self, item: &hir::Item) {
11431136
self.check_sane_privacy(item);
1144-
if self.in_block {
1145-
self.check_all_inherited(item);
1146-
}
1147-
1148-
let orig_in_block = self.in_block;
1149-
1150-
// Modules turn privacy back on, otherwise we inherit
1151-
self.in_block = if let hir::ItemMod(..) = item.node { false } else { orig_in_block };
1152-
11531137
intravisit::walk_item(self, item);
1154-
self.in_block = orig_in_block;
1155-
}
1156-
1157-
fn visit_block(&mut self, b: &'v hir::Block) {
1158-
let orig_in_block = replace(&mut self.in_block, true);
1159-
intravisit::walk_block(self, b);
1160-
self.in_block = orig_in_block;
11611138
}
11621139
}
11631140

11641141
impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> {
1165-
/// Validates all of the visibility qualifiers placed on the item given. This
1166-
/// ensures that there are no extraneous qualifiers that don't actually do
1167-
/// anything. In theory these qualifiers wouldn't parse, but that may happen
1168-
/// later on down the road...
1142+
/// Validate that items that shouldn't have visibility qualifiers don't have them.
1143+
/// Such qualifiers can be set by syntax extensions even if the parser doesn't allow them,
1144+
/// so we check things like variant fields too.
11691145
fn check_sane_privacy(&self, item: &hir::Item) {
11701146
let check_inherited = |sp, vis, note: &str| {
11711147
if vis != hir::Inherited {
@@ -1179,13 +1155,12 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> {
11791155
};
11801156

11811157
match item.node {
1182-
// implementations of traits don't need visibility qualifiers because
1183-
// that's controlled by having the trait in scope.
11841158
hir::ItemImpl(_, _, _, Some(..), _, ref impl_items) => {
11851159
check_inherited(item.span, item.vis,
11861160
"visibility qualifiers have no effect on trait impls");
11871161
for impl_item in impl_items {
1188-
check_inherited(impl_item.span, impl_item.vis, "");
1162+
check_inherited(impl_item.span, impl_item.vis,
1163+
"visibility qualifiers have no effect on trait impl items");
11891164
}
11901165
}
11911166
hir::ItemImpl(_, _, _, None, _, _) => {
@@ -1200,41 +1175,15 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> {
12001175
check_inherited(item.span, item.vis,
12011176
"place qualifiers on individual functions instead");
12021177
}
1203-
hir::ItemStruct(..) | hir::ItemEnum(..) | hir::ItemTrait(..) |
1204-
hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) |
1205-
hir::ItemMod(..) | hir::ItemExternCrate(..) |
1206-
hir::ItemUse(..) | hir::ItemTy(..) => {}
1207-
}
1208-
}
1209-
1210-
/// When inside of something like a function or a method, visibility has no
1211-
/// control over anything so this forbids any mention of any visibility
1212-
fn check_all_inherited(&self, item: &hir::Item) {
1213-
let check_inherited = |sp, vis| {
1214-
if vis != hir::Inherited {
1215-
span_err!(self.tcx.sess, sp, E0447,
1216-
"visibility has no effect inside functions or block expressions");
1217-
}
1218-
};
1219-
1220-
check_inherited(item.span, item.vis);
1221-
match item.node {
1222-
hir::ItemImpl(_, _, _, _, _, ref impl_items) => {
1223-
for impl_item in impl_items {
1224-
check_inherited(impl_item.span, impl_item.vis);
1225-
}
1226-
}
1227-
hir::ItemForeignMod(ref fm) => {
1228-
for fi in &fm.items {
1229-
check_inherited(fi.span, fi.vis);
1230-
}
1231-
}
1232-
hir::ItemStruct(ref vdata, _) => {
1233-
for f in vdata.fields() {
1234-
check_inherited(f.span, f.node.kind.visibility());
1178+
hir::ItemEnum(ref def, _) => {
1179+
for variant in &def.variants {
1180+
for field in variant.node.data.fields() {
1181+
check_inherited(field.span, field.node.kind.visibility(),
1182+
"visibility qualifiers have no effect on variant fields");
1183+
}
12351184
}
12361185
}
1237-
hir::ItemDefaultImpl(..) | hir::ItemEnum(..) | hir::ItemTrait(..) |
1186+
hir::ItemStruct(..) | hir::ItemTrait(..) |
12381187
hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) |
12391188
hir::ItemMod(..) | hir::ItemExternCrate(..) |
12401189
hir::ItemUse(..) | hir::ItemTy(..) => {}
@@ -1821,13 +1770,9 @@ pub fn check_crate(tcx: &ty::ctxt,
18211770

18221771
let krate = tcx.map.krate();
18231772

1824-
// Sanity check to make sure that all privacy usage and controls are
1825-
// reasonable.
1826-
let mut visitor = SanePrivacyVisitor {
1827-
tcx: tcx,
1828-
in_block: false,
1829-
};
1830-
intravisit::walk_crate(&mut visitor, krate);
1773+
// Sanity check to make sure that all privacy usage is reasonable.
1774+
let mut visitor = SanePrivacyVisitor { tcx: tcx };
1775+
krate.visit_all_items(&mut visitor);
18311776

18321777
// Figure out who everyone's parent is
18331778
let mut visitor = ParentVisitor {

‎src/test/compile-fail/privacy-sanity.rs

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,74 +40,60 @@ pub extern "C" { //~ ERROR unnecessary visibility qualifier
4040

4141
const MAIN: u8 = {
4242
trait MarkerTr {}
43-
pub trait Tr { //~ ERROR visibility has no effect inside functions or block
43+
pub trait Tr {
4444
fn f();
4545
const C: u8;
4646
type T;
4747
}
48-
pub struct S { //~ ERROR visibility has no effect inside functions or block
49-
pub a: u8 //~ ERROR visibility has no effect inside functions or block
48+
pub struct S {
49+
pub a: u8
5050
}
51-
struct Ts(pub u8); //~ ERROR visibility has no effect inside functions or block
51+
struct Ts(pub u8);
5252

5353
pub impl MarkerTr for .. {} //~ ERROR unnecessary visibility qualifier
54-
//~^ ERROR visibility has no effect inside functions or block
5554
pub impl Tr for S { //~ ERROR unnecessary visibility qualifier
56-
//~^ ERROR visibility has no effect inside functions or block
5755
pub fn f() {} //~ ERROR unnecessary visibility qualifier
58-
//~^ ERROR visibility has no effect inside functions or block
5956
pub const C: u8 = 0; //~ ERROR unnecessary visibility qualifier
60-
//~^ ERROR visibility has no effect inside functions or block
6157
pub type T = u8; //~ ERROR unnecessary visibility qualifier
62-
//~^ ERROR visibility has no effect inside functions or block
6358
}
6459
pub impl S { //~ ERROR unnecessary visibility qualifier
65-
//~^ ERROR visibility has no effect inside functions or block
66-
pub fn f() {} //~ ERROR visibility has no effect inside functions or block
67-
pub const C: u8 = 0; //~ ERROR visibility has no effect inside functions or block
68-
// pub type T = u8; // ERROR visibility has no effect inside functions or block
60+
pub fn f() {}
61+
pub const C: u8 = 0;
62+
// pub type T = u8;
6963
}
7064
pub extern "C" { //~ ERROR unnecessary visibility qualifier
71-
//~^ ERROR visibility has no effect inside functions or block
72-
pub fn f(); //~ ERROR visibility has no effect inside functions or block
73-
pub static St: u8; //~ ERROR visibility has no effect inside functions or block
65+
pub fn f();
66+
pub static St: u8;
7467
}
7568

7669
0
7770
};
7871

7972
fn main() {
8073
trait MarkerTr {}
81-
pub trait Tr { //~ ERROR visibility has no effect inside functions or block
74+
pub trait Tr {
8275
fn f();
8376
const C: u8;
8477
type T;
8578
}
86-
pub struct S { //~ ERROR visibility has no effect inside functions or block
87-
pub a: u8 //~ ERROR visibility has no effect inside functions or block
79+
pub struct S {
80+
pub a: u8
8881
}
89-
struct Ts(pub u8); //~ ERROR visibility has no effect inside functions or block
82+
struct Ts(pub u8);
9083

9184
pub impl MarkerTr for .. {} //~ ERROR unnecessary visibility qualifier
92-
//~^ ERROR visibility has no effect inside functions or block
9385
pub impl Tr for S { //~ ERROR unnecessary visibility qualifier
94-
//~^ ERROR visibility has no effect inside functions or block
9586
pub fn f() {} //~ ERROR unnecessary visibility qualifier
96-
//~^ ERROR visibility has no effect inside functions or block
9787
pub const C: u8 = 0; //~ ERROR unnecessary visibility qualifier
98-
//~^ ERROR visibility has no effect inside functions or block
9988
pub type T = u8; //~ ERROR unnecessary visibility qualifier
100-
//~^ ERROR visibility has no effect inside functions or block
10189
}
10290
pub impl S { //~ ERROR unnecessary visibility qualifier
103-
//~^ ERROR visibility has no effect inside functions or block
104-
pub fn f() {} //~ ERROR visibility has no effect inside functions or block
105-
pub const C: u8 = 0; //~ ERROR visibility has no effect inside functions or block
106-
// pub type T = u8; // ERROR visibility has no effect inside functions or block
91+
pub fn f() {}
92+
pub const C: u8 = 0;
93+
// pub type T = u8;
10794
}
10895
pub extern "C" { //~ ERROR unnecessary visibility qualifier
109-
//~^ ERROR visibility has no effect inside functions or block
110-
pub fn f(); //~ ERROR visibility has no effect inside functions or block
111-
pub static St: u8; //~ ERROR visibility has no effect inside functions or block
96+
pub fn f();
97+
pub static St: u8;
11298
}
11399
}

‎src/test/compile-fail/unnecessary-private.rs

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎src/test/run-pass/issue-31776.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
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+
// Various scenarios in which `pub` is required in blocks
12+
13+
struct S;
14+
15+
mod m {
16+
fn f() {
17+
impl ::S {
18+
pub fn s(&self) {}
19+
}
20+
}
21+
}
22+
23+
// ------------------------------------------------------
24+
25+
pub trait Tr {
26+
type A;
27+
}
28+
pub struct S1;
29+
30+
fn f() {
31+
pub struct Z;
32+
33+
impl ::Tr for ::S1 {
34+
type A = Z; // Private-in-public error unless `struct Z` is pub
35+
}
36+
}
37+
38+
// ------------------------------------------------------
39+
40+
trait Tr1 {
41+
type A;
42+
fn pull(&self) -> Self::A;
43+
}
44+
struct S2;
45+
46+
mod m1 {
47+
fn f() {
48+
struct Z {
49+
pub field: u8
50+
}
51+
52+
impl ::Tr1 for ::S2 {
53+
type A = Z;
54+
fn pull(&self) -> Self::A { Z{field: 10} }
55+
}
56+
}
57+
}
58+
59+
// ------------------------------------------------------
60+
61+
fn main() {
62+
S.s(); // Privacy error, unless `fn s` is pub
63+
let a = S2.pull().field; // Privacy error unless `field: u8` is pub
64+
}

0 commit comments

Comments
 (0)