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.

Sign up for GitHub

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

Jump to bottom

Make privacy visitor use types more (instead of HIR) #113671

Merged
merged 7 commits into from
Feb 9, 2024
Merged

Make privacy visitor use types more (instead of HIR) #113671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0f323b2
Associated types in traits don't necessarily have a type that we can …
oli-obk Feb 7, 2024
0b97d18
extern types don't have any types to visit
oli-obk Feb 7, 2024
b998b51
Actually walk fields of Adt definitions
oli-obk Feb 7, 2024
4389a1c
Stop using `hir_ty_to_ty` in rustc_privacy
oli-obk Jul 18, 2023
5c25de6
Remove now-useless method override
oli-obk Feb 7, 2024
e867886
Remove dead code
oli-obk Feb 7, 2024
d80d7ea
Add some tests for associated type normalization edge cases
oli-obk Feb 8, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4374,6 +4374,7 @@ dependencies = [
"rustc_middle",
"rustc_session",
"rustc_span",
"rustc_ty_utils",
"tracing",
]

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_privacy/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ rustc_macros = { path = "../rustc_macros" }
rustc_middle = { path = "../rustc_middle" }
rustc_session = { path = "../rustc_session" }
rustc_span = { path = "../rustc_span" }
rustc_ty_utils = { path = "../rustc_ty_utils" }
tracing = "0.1"
# tidy-alphabetical-end
111 changes: 50 additions & 61 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId, CRATE_DEF_ID};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, PatKind};
use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, ItemKind, PatKind};
use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level};
use rustc_middle::query::Providers;
use rustc_middle::ty::GenericArgs;
Expand Down Expand Up @@ -98,9 +98,6 @@ trait DefIdVisitor<'tcx> {
fn visit_trait(&mut self, trait_ref: TraitRef<'tcx>) -> ControlFlow<Self::BreakTy> {
self.skeleton().visit_trait(trait_ref)
}
fn visit_projection_ty(&mut self, projection: ty::AliasTy<'tcx>) -> ControlFlow<Self::BreakTy> {
self.skeleton().visit_projection_ty(projection)
}
fn visit_predicates(
&mut self,
predicates: ty::GenericPredicates<'tcx>,
Expand Down Expand Up @@ -173,6 +170,10 @@ where
{
type BreakTy = V::BreakTy;

fn visit_predicate(&mut self, p: ty::Predicate<'tcx>) -> ControlFlow<Self::BreakTy> {
self.visit_clause(p.as_clause().unwrap())
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
}

fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<V::BreakTy> {
let tcx = self.def_id_visitor.tcx();
// GenericArgs are not visited here because they are visited below
Expand Down Expand Up @@ -1076,6 +1077,14 @@ impl<'tcx> TypePrivacyVisitor<'tcx> {
}
}

impl<'tcx> rustc_ty_utils::sig_types::SpannedTypeVisitor<'tcx> for TypePrivacyVisitor<'tcx> {
type BreakTy = ();
fn visit(&mut self, span: Span, value: impl TypeVisitable<TyCtxt<'tcx>>) -> ControlFlow<()> {
self.span = span;
value.visit_with(&mut self.skeleton())
}
}

impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
fn visit_nested_body(&mut self, body_id: hir::BodyId) {
let old_maybe_typeck_results =
Expand All @@ -1086,75 +1095,36 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {

fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
self.span = hir_ty.span;
if let Some(typeck_results) = self.maybe_typeck_results {
// Types in bodies.
if self.visit(typeck_results.node_type(hir_ty.hir_id)).is_break() {
return;
}
} else {
// Types in signatures.
// FIXME: This is very ineffective. Ideally each HIR type should be converted
// into a semantic type only once and the result should be cached somehow.
if self.visit(rustc_hir_analysis::hir_ty_to_ty(self.tcx, hir_ty)).is_break() {
return;
}
if self
.visit(
self.maybe_typeck_results
.unwrap_or_else(|| span_bug!(hir_ty.span, "`hir::Ty` outside of a body"))
.node_type(hir_ty.hir_id),
)
.is_break()
{
return;
}

intravisit::walk_ty(self, hir_ty);
}

fn visit_infer(&mut self, inf: &'tcx hir::InferArg) {
self.span = inf.span;
if let Some(typeck_results) = self.maybe_typeck_results {
if let Some(ty) = typeck_results.node_type_opt(inf.hir_id) {
if self.visit(ty).is_break() {
return;
}
} else {
// FIXME: check types of const infers here.
if let Some(ty) = self
.maybe_typeck_results
.unwrap_or_else(|| span_bug!(inf.span, "`hir::InferArg` outside of a body"))
.node_type_opt(inf.hir_id)
{
if self.visit(ty).is_break() {
return;
}
} else {
span_bug!(self.span, "`hir::InferArg` outside of a body");
// FIXME: check types of const infers here.
}
intravisit::walk_inf(self, inf);
}

fn visit_trait_ref(&mut self, trait_ref: &'tcx hir::TraitRef<'tcx>) {
self.span = trait_ref.path.span;
if self.maybe_typeck_results.is_some() {
// Privacy of traits in bodies is checked as a part of trait object types.
} else {
let bounds = rustc_hir_analysis::hir_trait_to_predicates(
self.tcx,
trait_ref,
// NOTE: This isn't really right, but the actual type doesn't matter here. It's
// just required by `ty::TraitRef`.
self.tcx.types.never,
);

for (clause, _) in bounds.clauses() {
match clause.kind().skip_binder() {
ty::ClauseKind::Trait(trait_predicate) => {
if self.visit_trait(trait_predicate.trait_ref).is_break() {
return;
}
}
ty::ClauseKind::Projection(proj_predicate) => {
let term = self.visit(proj_predicate.term);
if term.is_break()
|| self.visit_projection_ty(proj_predicate.projection_ty).is_break()
{
return;
}
}
_ => {}
}
}
}

intravisit::walk_trait_ref(self, trait_ref);
}

// Check types of expressions
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
if self.check_expr_pat_type(expr.hir_id, expr.span) {
Expand Down Expand Up @@ -1727,7 +1697,26 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
// inferred types of expressions and patterns.
let span = tcx.def_span(module_def_id);
let mut visitor = TypePrivacyVisitor { tcx, module_def_id, maybe_typeck_results: None, span };
tcx.hir().visit_item_likes_in_module(module_def_id, &mut visitor);

let module = tcx.hir_module_items(module_def_id);
for def_id in module.definitions() {
rustc_ty_utils::sig_types::walk_types(tcx, def_id, &mut visitor);

if let Some(body_id) = tcx.hir().maybe_body_owned_by(def_id) {
visitor.visit_nested_body(body_id);
}
}

for id in module.items() {
if let ItemKind::Impl(i) = tcx.hir().item(id).kind {
if let Some(item) = i.of_trait {
let trait_ref = tcx.impl_trait_ref(id.owner_id.def_id).unwrap();
let trait_ref = trait_ref.instantiate_identity();
visitor.span = item.path.span;
visitor.visit_def_id(trait_ref.def_id, "trait", &trait_ref.print_only_trait_path());
}
}
}
}

fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ mod layout_sanity_check;
mod needs_drop;
mod opaque_types;
mod representability;
mod sig_types;
pub mod sig_types;
mod structural_match;
mod ty;

Expand Down
28 changes: 18 additions & 10 deletions compiler/rustc_ty_utils/src/sig_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
use std::ops::ControlFlow;

use rustc_hir::{def::DefKind, def_id::LocalDefId};
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Span;
use rustc_type_ir::visit::TypeVisitable;

pub(crate) trait SpannedTypeVisitor<'tcx> {
pub trait SpannedTypeVisitor<'tcx> {
type BreakTy = !;
fn visit(
&mut self,
Expand All @@ -17,7 +17,7 @@ pub(crate) trait SpannedTypeVisitor<'tcx> {
) -> ControlFlow<Self::BreakTy>;
}

pub(crate) fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
pub fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
tcx: TyCtxt<'tcx>,
item: LocalDefId,
visitor: &mut V,
Expand All @@ -42,11 +42,10 @@ pub(crate) fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
DefKind::TyAlias {..} | DefKind::AssocTy |
// Walk over the type of the item
DefKind::Static(_) | DefKind::Const | DefKind::AssocConst | DefKind::AnonConst => {
let span = match tcx.hir_node_by_def_id(item).ty() {
Some(ty) => ty.span,
_ => tcx.def_span(item),
};
visitor.visit(span, tcx.type_of(item).instantiate_identity());
if let Some(ty) = tcx.hir_node_by_def_id(item).ty() {
// Associated types in traits don't necessarily have a type that we can visit
visitor.visit(ty.span, tcx.type_of(item).instantiate_identity())?;
}
for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
visitor.visit(span, pred)?;
}
Expand All @@ -59,7 +58,16 @@ pub(crate) fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
// Look at field types
DefKind::Struct | DefKind::Union | DefKind::Enum => {
let span = tcx.def_ident_span(item).unwrap();
visitor.visit(span, tcx.type_of(item).instantiate_identity());
let ty = tcx.type_of(item).instantiate_identity();
visitor.visit(span, ty);
let ty::Adt(def, args) = ty.kind() else {
span_bug!(span, "invalid type for {kind:?}: {:#?}", ty.kind())
};
for field in def.all_fields() {
let span = tcx.def_ident_span(field.did).unwrap();
let ty = field.ty(tcx, args);
visitor.visit(span, ty);
}
for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
visitor.visit(span, pred)?;
}
Expand Down Expand Up @@ -89,7 +97,6 @@ pub(crate) fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
}
}
| DefKind::Variant
| DefKind::ForeignTy
| DefKind::TyParam
| DefKind::ConstParam
| DefKind::Ctor(_, _)
Expand All @@ -103,6 +110,7 @@ pub(crate) fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
// These don't have any types.
| DefKind::ExternCrate
| DefKind::ForeignMod
| DefKind::ForeignTy
| DefKind::Macro(_)
| DefKind::GlobalAsm
| DefKind::Mod
Expand Down
8 changes: 0 additions & 8 deletions tests/ui/dyn-keyword/dyn-2018-edition-lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ fn function(x: &SomeTrait, y: Box<SomeTrait>) {
//~| WARN this is accepted in the current edition
//~| ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
//~| ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
//~| ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
//~| ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
//~| ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
let _x: &SomeTrait = todo!();
//~^ ERROR trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
Expand Down
60 changes: 2 additions & 58 deletions tests/ui/dyn-keyword/dyn-2018-edition-lint.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ LL | fn function(x: &SomeTrait, y: Box<dyn SomeTrait>) {
| +++

error: trait objects without an explicit `dyn` are deprecated
--> $DIR/dyn-2018-edition-lint.rs:17:14
--> $DIR/dyn-2018-edition-lint.rs:9:14
|
LL | let _x: &SomeTrait = todo!();
| ^^^^^^^^^
Expand All @@ -42,61 +42,5 @@ help: use `dyn`
LL | let _x: &dyn SomeTrait = todo!();
| +++

error: trait objects without an explicit `dyn` are deprecated
--> $DIR/dyn-2018-edition-lint.rs:4:17
|
LL | fn function(x: &SomeTrait, y: Box<SomeTrait>) {
| ^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn function(x: &dyn SomeTrait, y: Box<SomeTrait>) {
| +++

error: trait objects without an explicit `dyn` are deprecated
--> $DIR/dyn-2018-edition-lint.rs:4:17
|
LL | fn function(x: &SomeTrait, y: Box<SomeTrait>) {
| ^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn function(x: &dyn SomeTrait, y: Box<SomeTrait>) {
| +++

error: trait objects without an explicit `dyn` are deprecated
--> $DIR/dyn-2018-edition-lint.rs:4:35
|
LL | fn function(x: &SomeTrait, y: Box<SomeTrait>) {
| ^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn function(x: &SomeTrait, y: Box<dyn SomeTrait>) {
| +++

error: trait objects without an explicit `dyn` are deprecated
--> $DIR/dyn-2018-edition-lint.rs:4:35
|
LL | fn function(x: &SomeTrait, y: Box<SomeTrait>) {
| ^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn function(x: &SomeTrait, y: Box<dyn SomeTrait>) {
| +++

error: aborting due to 7 previous errors
error: aborting due to 3 previous errors

4 changes: 0 additions & 4 deletions tests/ui/impl-trait/fresh-lifetime-from-bare-trait-obj-114664.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

fn ice() -> impl AsRef<Fn(&())> {
//~^ WARN trait objects without an explicit `dyn` are deprecated
//~| WARN trait objects without an explicit `dyn` are deprecated
//~| WARN trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
//~| WARN this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
//~| WARN this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
Foo
}
Expand Down
30 changes: 1 addition & 29 deletions tests/ui/impl-trait/fresh-lifetime-from-bare-trait-obj-114664.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,5 @@ help: use `dyn`
LL | fn ice() -> impl AsRef<dyn Fn(&())> {
| +++

warning: trait objects without an explicit `dyn` are deprecated
--> $DIR/fresh-lifetime-from-bare-trait-obj-114664.rs:5:24
|
LL | fn ice() -> impl AsRef<Fn(&())> {
| ^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn ice() -> impl AsRef<dyn Fn(&())> {
| +++

warning: trait objects without an explicit `dyn` are deprecated
--> $DIR/fresh-lifetime-from-bare-trait-obj-114664.rs:5:24
|
LL | fn ice() -> impl AsRef<Fn(&())> {
| ^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: use `dyn`
|
LL | fn ice() -> impl AsRef<dyn Fn(&())> {
| +++

warning: 3 warnings emitted
warning: 1 warning emitted

Loading
Loading