Skip to content

Commit

Permalink
Rollup merge of #113671 - oli-obk:normalize_weak_tys, r=petrochenkov
Browse files Browse the repository at this point in the history
Make privacy visitor use types more (instead of HIR)

r? ``@petrochenkov``

This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder).

The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible.

[context can be found on zulip](https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/weak.20type.20aliases.20and.20privacy)
  • Loading branch information
matthiaskrgr authored Feb 9, 2024
2 parents 972452c + d80d7ea commit f41d0d9
Show file tree
Hide file tree
Showing 36 changed files with 400 additions and 582 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
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
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
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())
}

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
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
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
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
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

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
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

0 comments on commit f41d0d9

Please sign in to comment.