Skip to content

Commit d815717

Browse files
rust-bors[bot]rust-bors[bot]
authored
Auto merge of #147486 - petrochenkov:optpriv, r=<try>
[PERF] privacy: Visit DefIds once in DefIdVisitorSkeleton
2 parents 0b278a5 + 1833298 commit d815717

File tree

7 files changed

+52
-68
lines changed

7 files changed

+52
-68
lines changed

‎compiler/rustc_privacy/src/lib.rs‎

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,16 @@ pub trait DefIdVisitor<'tcx> {
7575
}
7676

7777
fn tcx(&self) -> TyCtxt<'tcx>;
78+
/// NOTE: Def-id visiting should be idempotent, because `DefIdVisitorSkeleton` will avoid
79+
/// visiting duplicate def-ids. All the current visitors follow this rule.
7880
fn visit_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display)
7981
-> Self::Result;
8082

8183
/// Not overridden, but used to actually visit types and traits.
8284
fn skeleton(&mut self) -> DefIdVisitorSkeleton<'_, 'tcx, Self> {
8385
DefIdVisitorSkeleton {
8486
def_id_visitor: self,
85-
visited_opaque_tys: Default::default(),
87+
visited_tys: Default::default(),
8688
dummy: Default::default(),
8789
}
8890
}
@@ -102,7 +104,7 @@ pub trait DefIdVisitor<'tcx> {
102104

103105
pub struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> {
104106
def_id_visitor: &'v mut V,
105-
visited_opaque_tys: FxHashSet<DefId>,
107+
visited_tys: FxHashSet<Ty<'tcx>>,
106108
dummy: PhantomData<TyCtxt<'tcx>>,
107109
}
108110

@@ -190,6 +192,9 @@ where
190192
| ty::Closure(def_id, ..)
191193
| ty::CoroutineClosure(def_id, ..)
192194
| ty::Coroutine(def_id, ..) => {
195+
if !self.visited_tys.insert(ty) {
196+
return V::Result::output();
197+
}
193198
try_visit!(self.def_id_visitor.visit_def_id(def_id, "type", &ty));
194199
if V::SHALLOW {
195200
return V::Result::output();
@@ -212,6 +217,9 @@ where
212217
}
213218
}
214219
ty::Alias(kind @ (ty::Inherent | ty::Free | ty::Projection), data) => {
220+
if !self.visited_tys.insert(ty) {
221+
return V::Result::output();
222+
}
215223
if self.def_id_visitor.skip_assoc_tys() {
216224
// Visitors searching for minimal visibility/reachability want to
217225
// conservatively approximate associated types like `Type::Alias`
@@ -243,6 +251,9 @@ where
243251
};
244252
}
245253
ty::Dynamic(predicates, ..) => {
254+
if !self.visited_tys.insert(ty) {
255+
return V::Result::output();
256+
}
246257
// All traits in the list are considered the "primary" part of the type
247258
// and are visited by shallow visitors.
248259
for predicate in predicates {
@@ -258,39 +269,45 @@ where
258269
}
259270
}
260271
ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) => {
261-
// Skip repeated `Opaque`s to avoid infinite recursion.
262-
if self.visited_opaque_tys.insert(def_id) {
263-
// The intent is to treat `impl Trait1 + Trait2` identically to
264-
// `dyn Trait1 + Trait2`. Therefore we ignore def-id of the opaque type itself
265-
// (it either has no visibility, or its visibility is insignificant, like
266-
// visibilities of type aliases) and recurse into bounds instead to go
267-
// through the trait list (default type visitor doesn't visit those traits).
268-
// All traits in the list are considered the "primary" part of the type
269-
// and are visited by shallow visitors.
270-
try_visit!(self.visit_clauses(tcx.explicit_item_bounds(def_id).skip_binder()));
272+
if !self.visited_tys.insert(ty) {
273+
return V::Result::output();
271274
}
275+
// The intent is to treat `impl Trait1 + Trait2` identically to
276+
// `dyn Trait1 + Trait2`. Therefore we ignore def-id of the opaque type itself
277+
// (it either has no visibility, or its visibility is insignificant, like
278+
// visibilities of type aliases) and recurse into bounds instead to go
279+
// through the trait list (default type visitor doesn't visit those traits).
280+
// All traits in the list are considered the "primary" part of the type
281+
// and are visited by shallow visitors.
282+
try_visit!(self.visit_clauses(tcx.explicit_item_bounds(def_id).skip_binder()));
272283
}
273-
// These types don't have their own def-ids (but may have subcomponents
274-
// with def-ids that should be visited recursively).
284+
// These types have neither their own def-ids nor subcomponents.
275285
ty::Bool
276286
| ty::Char
277287
| ty::Int(..)
278288
| ty::Uint(..)
279289
| ty::Float(..)
280290
| ty::Str
281291
| ty::Never
282-
| ty::Array(..)
292+
| ty::Bound(..)
293+
| ty::Param(..) => return V::Result::output(),
294+
295+
// These types don't have their own def-ids (but may have subcomponents
296+
// with def-ids that should be visited recursively).
297+
ty::Array(..)
283298
| ty::Slice(..)
284299
| ty::Tuple(..)
285300
| ty::RawPtr(..)
286301
| ty::Ref(..)
287302
| ty::Pat(..)
288303
| ty::FnPtr(..)
289304
| ty::UnsafeBinder(_)
290-
| ty::Param(..)
291-
| ty::Bound(..)
292305
| ty::Error(_)
293-
| ty::CoroutineWitness(..) => {}
306+
| ty::CoroutineWitness(..) => {
307+
if !self.visited_tys.insert(ty) {
308+
return V::Result::output();
309+
}
310+
}
294311
ty::Placeholder(..) | ty::Infer(..) => {
295312
bug!("unexpected type: {:?}", ty)
296313
}
@@ -923,7 +940,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
923940

924941
// Checks that a field in a struct constructor (expression or pattern) is accessible.
925942
fn check_field(
926-
&mut self,
943+
&self,
927944
hir_id: hir::HirId, // ID of the field use
928945
use_ctxt: Span, // syntax context of the field name at the use site
929946
def: ty::AdtDef<'tcx>, // definition of the struct or enum
@@ -941,7 +958,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
941958

942959
// Checks that a field in a struct constructor (expression or pattern) is accessible.
943960
fn emit_unreachable_field_error(
944-
&mut self,
961+
&self,
945962
fields: Vec<(Symbol, Span, bool /* field is present */)>,
946963
def: ty::AdtDef<'tcx>, // definition of the struct or enum
947964
update_syntax: Option<Span>,
@@ -1004,7 +1021,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
10041021
}
10051022

10061023
fn check_expanded_fields(
1007-
&mut self,
1024+
&self,
10081025
adt: ty::AdtDef<'tcx>,
10091026
variant: &'tcx ty::VariantDef,
10101027
fields: &[hir::ExprField<'tcx>],
@@ -1142,7 +1159,7 @@ impl<'tcx> TypePrivacyVisitor<'tcx> {
11421159
result.is_break()
11431160
}
11441161

1145-
fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
1162+
fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
11461163
let is_error = !self.item_is_accessible(def_id);
11471164
if is_error {
11481165
self.tcx.dcx().emit_err(ItemIsPrivate { span: self.span, kind, descr: descr.into() });
@@ -1401,7 +1418,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
14011418
self
14021419
}
14031420

1404-
fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
1421+
fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
14051422
if self.leaks_private_dep(def_id) {
14061423
self.tcx.emit_node_span_lint(
14071424
lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES,

‎tests/ui/const-generics/generic_const_exprs/eval-privacy.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ where
1616
{
1717
type AssocTy = Const<{ my_const_fn(U) }>;
1818
//~^ ERROR private type
19-
//~| ERROR private type
2019
fn assoc_fn() -> Self::AssocTy {
2120
Const
2221
}

‎tests/ui/const-generics/generic_const_exprs/eval-privacy.stderr‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,6 @@ LL | type AssocTy = Const<{ my_const_fn(U) }>;
77
LL | const fn my_const_fn(val: u8) -> u8 {
88
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
99

10-
error[E0446]: private type `fn(u8) -> u8 {my_const_fn}` in public interface
11-
--> $DIR/eval-privacy.rs:17:5
12-
|
13-
LL | type AssocTy = Const<{ my_const_fn(U) }>;
14-
| ^^^^^^^^^^^^ can't leak private type
15-
...
16-
LL | const fn my_const_fn(val: u8) -> u8 {
17-
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
18-
|
19-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
20-
21-
error: aborting due to 2 previous errors
10+
error: aborting due to 1 previous error
2211

2312
For more information about this error, try `rustc --explain E0446`.

‎tests/ui/privacy/pub-priv-dep/pub-priv1.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ impl From<PublicWithStdImpl> for OtherType {
145145
//~| ERROR type `OtherType` from private dependency 'priv_dep' in public interface
146146
fn from(val: PublicWithStdImpl) -> Self { Self }
147147
//~^ ERROR type `OtherType` from private dependency 'priv_dep' in public interface
148-
//~| ERROR type `OtherType` from private dependency 'priv_dep' in public interface
149148
}
150149

151150
pub struct AllowedPrivType {

‎tests/ui/privacy/pub-priv-dep/pub-priv1.stderr‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,55 +11,55 @@ LL | #![deny(exported_private_dependencies)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1212

1313
error: macro `m` from private dependency 'priv_dep' is re-exported
14-
--> $DIR/pub-priv1.rs:156:9
14+
--> $DIR/pub-priv1.rs:155:9
1515
|
1616
LL | pub use priv_dep::m;
1717
| ^^^^^^^^^^^
1818

1919
error: macro `fn_like` from private dependency 'pm' is re-exported
20-
--> $DIR/pub-priv1.rs:158:9
20+
--> $DIR/pub-priv1.rs:157:9
2121
|
2222
LL | pub use pm::fn_like;
2323
| ^^^^^^^^^^^
2424

2525
error: derive macro `PmDerive` from private dependency 'pm' is re-exported
26-
--> $DIR/pub-priv1.rs:160:9
26+
--> $DIR/pub-priv1.rs:159:9
2727
|
2828
LL | pub use pm::PmDerive;
2929
| ^^^^^^^^^^^^
3030

3131
error: attribute macro `pm_attr` from private dependency 'pm' is re-exported
32-
--> $DIR/pub-priv1.rs:162:9
32+
--> $DIR/pub-priv1.rs:161:9
3333
|
3434
LL | pub use pm::pm_attr;
3535
| ^^^^^^^^^^^
3636

3737
error: variant `V1` from private dependency 'priv_dep' is re-exported
38-
--> $DIR/pub-priv1.rs:165:9
38+
--> $DIR/pub-priv1.rs:164:9
3939
|
4040
LL | pub use priv_dep::E::V1;
4141
| ^^^^^^^^^^^^^^^
4242

4343
error: type alias `Unit` from private dependency 'priv_dep' is re-exported
44-
--> $DIR/pub-priv1.rs:168:9
44+
--> $DIR/pub-priv1.rs:167:9
4545
|
4646
LL | pub use priv_dep::Unit;
4747
| ^^^^^^^^^^^^^^
4848

4949
error: type alias `PubPub` from private dependency 'priv_dep' is re-exported
50-
--> $DIR/pub-priv1.rs:170:9
50+
--> $DIR/pub-priv1.rs:169:9
5151
|
5252
LL | pub use priv_dep::PubPub;
5353
| ^^^^^^^^^^^^^^^^
5454

5555
error: type alias `PubPriv` from private dependency 'priv_dep' is re-exported
56-
--> $DIR/pub-priv1.rs:172:9
56+
--> $DIR/pub-priv1.rs:171:9
5757
|
5858
LL | pub use priv_dep::PubPriv;
5959
| ^^^^^^^^^^^^^^^^^
6060

6161
error: struct `Renamed` from private dependency 'priv_dep' is re-exported
62-
--> $DIR/pub-priv1.rs:174:9
62+
--> $DIR/pub-priv1.rs:173:9
6363
|
6464
LL | pub use priv_dep::OtherType as Renamed;
6565
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -248,13 +248,5 @@ error: type `OtherType` from private dependency 'priv_dep' in public interface
248248
LL | fn from(val: PublicWithStdImpl) -> Self { Self }
249249
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
250250

251-
error: type `OtherType` from private dependency 'priv_dep' in public interface
252-
--> $DIR/pub-priv1.rs:146:5
253-
|
254-
LL | fn from(val: PublicWithStdImpl) -> Self { Self }
255-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
256-
|
257-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
258-
259-
error: aborting due to 41 previous errors
251+
error: aborting due to 40 previous errors
260252

‎tests/ui/privacy/where-priv-type.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ where
6565
{
6666
type AssocTy = Const<{ my_const_fn(U) }>;
6767
//~^ ERROR private type
68-
//~| ERROR private type
6968
fn assoc_fn() -> Self::AssocTy {
7069
Const
7170
}

‎tests/ui/privacy/where-priv-type.stderr‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,6 @@ LL | type AssocTy = Const<{ my_const_fn(U) }>;
7777
LL | const fn my_const_fn(val: u8) -> u8 {
7878
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
7979

80-
error[E0446]: private type `fn(u8) -> u8 {my_const_fn}` in public interface
81-
--> $DIR/where-priv-type.rs:66:5
82-
|
83-
LL | type AssocTy = Const<{ my_const_fn(U) }>;
84-
| ^^^^^^^^^^^^ can't leak private type
85-
...
86-
LL | const fn my_const_fn(val: u8) -> u8 {
87-
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
88-
|
89-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
90-
91-
error: aborting due to 2 previous errors; 5 warnings emitted
80+
error: aborting due to 1 previous error; 5 warnings emitted
9281

9382
For more information about this error, try `rustc --explain E0446`.

0 commit comments

Comments
 (0)