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.

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

Assert that args are actually compatible with their generics, rather than just their count #123240

Merged
merged 5 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 27 additions & 17 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{DynKind, ToPredicate};
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};
use rustc_trait_selection::traits::error_reporting::report_object_safety_error;
use rustc_trait_selection::traits::{self, hir_ty_lowering_object_safety_violations};

Expand Down Expand Up @@ -228,12 +229,17 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
if arg == dummy_self.into() {
let param = &generics.params[index];
missing_type_params.push(param.name);
return Ty::new_misc_error(tcx).into();
Ty::new_misc_error(tcx).into()
} else if arg.walk().any(|arg| arg == dummy_self.into()) {
references_self = true;
return Ty::new_misc_error(tcx).into();
let guar = tcx.dcx().span_delayed_bug(
span,
"trait object trait bounds reference `Self`",
);
replace_dummy_self_with_error(tcx, arg, guar)
} else {
arg
}
arg
})
.collect();
let args = tcx.mk_args(&args);
Expand Down Expand Up @@ -288,18 +294,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let guar = tcx
.dcx()
.span_delayed_bug(span, "trait object projection bounds reference `Self`");
let args: Vec<_> = b
.projection_ty
.args
.iter()
.map(|arg| {
if arg.walk().any(|arg| arg == dummy_self.into()) {
return Ty::new_error(tcx, guar).into();
}
arg
})
.collect();
b.projection_ty.args = tcx.mk_args(&args);
b.projection_ty = replace_dummy_self_with_error(tcx, b.projection_ty, guar);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol you just keep improving things I've recently looked at. in this case, #123140. appreciate it 🙏

}

ty::ExistentialProjection::erase_self_ty(tcx, b)
Expand Down Expand Up @@ -357,3 +352,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Ty::new_dynamic(tcx, existential_predicates, region_bound, representation)
}
}

fn replace_dummy_self_with_error<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
tcx: TyCtxt<'tcx>,
t: T,
guar: ErrorGuaranteed,
) -> T {
t.fold_with(&mut BottomUpFolder {
tcx,
ty_op: |ty| {
if ty == tcx.types.trait_object_dummy_self { Ty::new_error(tcx, guar) } else { ty }
},
lt_op: |lt| lt,
ct_op: |ct| ct,
})
}
115 changes: 93 additions & 22 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1961,33 +1961,104 @@ impl<'tcx> TyCtxt<'tcx> {
if pred.kind() != binder { self.mk_predicate(binder) } else { pred }
}

#[inline(always)]
pub(crate) fn check_and_mk_args(
pub fn check_args_compatible(self, def_id: DefId, args: &'tcx [ty::GenericArg<'tcx>]) -> bool {
self.check_args_compatible_inner(def_id, args, false)
}

fn check_args_compatible_inner(
self,
_def_id: DefId,
args: impl IntoIterator<Item: Into<GenericArg<'tcx>>>,
) -> GenericArgsRef<'tcx> {
let args = args.into_iter().map(Into::into);
#[cfg(debug_assertions)]
def_id: DefId,
args: &'tcx [ty::GenericArg<'tcx>],
nested: bool,
) -> bool {
let generics = self.generics_of(def_id);

// IATs themselves have a weird arg setup (self + own args), but nested items *in* IATs
// (namely: opaques, i.e. ATPITs) do not.
Comment on lines +1976 to +1977
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a test that exercises this or could you post a snippet? haven't wrapped my head around that yet

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider an ITIATII (impl trait in associated type in inherent impl):

impl<T, U> Struct<T, U> {
  type Assoc = impl Trait;
}

The Assoc type will have substs [Struct<T, U>] but the impl Trait will have substs [T, U]. So if we recurse on the parent when checking the opaque in check_args_compatible_inner, we'll ICE.

let own_args = if !nested
&& let DefKind::AssocTy = self.def_kind(def_id)
&& let DefKind::Impl { of_trait: false } = self.def_kind(self.parent(def_id))
{
let generics = self.generics_of(_def_id);
if generics.params.len() + 1 != args.len() {
return false;
}

let n = if let DefKind::AssocTy = self.def_kind(_def_id)
&& let DefKind::Impl { of_trait: false } = self.def_kind(self.parent(_def_id))
if !matches!(args[0].unpack(), ty::GenericArgKind::Type(_)) {
return false;
}

&args[1..]
} else {
if generics.count() != args.len() {
return false;
}

let (parent_args, own_args) = args.split_at(generics.parent_count);

if let Some(parent) = generics.parent
&& !self.check_args_compatible_inner(parent, parent_args, true)
{
// If this is an inherent projection.
generics.params.len() + 1
} else {
generics.count()
};
assert_eq!(
(n, Some(n)),
args.size_hint(),
"wrong number of generic parameters for {_def_id:?}: {:?}",
args.collect::<Vec<_>>(),
);
return false;
}

own_args
};

for (param, arg) in std::iter::zip(&generics.params, own_args) {
match (&param.kind, arg.unpack()) {
(ty::GenericParamDefKind::Type { .. }, ty::GenericArgKind::Type(_))
| (ty::GenericParamDefKind::Lifetime, ty::GenericArgKind::Lifetime(_))
| (ty::GenericParamDefKind::Const { .. }, ty::GenericArgKind::Const(_)) => {}
_ => return false,
}
}
self.mk_args_from_iter(args)

true
}

/// With `cfg(debug_assertions)`, assert that args are compatible with their generics,
/// and print out the args if not.
pub fn debug_assert_args_compatible(self, def_id: DefId, args: &'tcx [ty::GenericArg<'tcx>]) {
if cfg!(debug_assertions) {
if !self.check_args_compatible(def_id, args) {
if let DefKind::AssocTy = self.def_kind(def_id)
&& let DefKind::Impl { of_trait: false } = self.def_kind(self.parent(def_id))
{
bug!(
"args not compatible with generics for {}: args={:#?}, generics={:#?}",
self.def_path_str(def_id),
args,
// Make `[Self, GAT_ARGS...]` (this could be simplified)
self.mk_args_from_iter(
[self.types.self_param.into()].into_iter().chain(
self.generics_of(def_id)
.own_args(ty::GenericArgs::identity_for_item(self, def_id))
.iter()
.copied()
)
)
);
} else {
bug!(
"args not compatible with generics for {}: args={:#?}, generics={:#?}",
self.def_path_str(def_id),
args,
ty::GenericArgs::identity_for_item(self, def_id)
);
}
}
}
}

#[inline(always)]
pub(crate) fn check_and_mk_args(
self,
def_id: DefId,
args: impl IntoIterator<Item: Into<GenericArg<'tcx>>>,
) -> GenericArgsRef<'tcx> {
let args = self.mk_args_from_iter(args.into_iter().map(Into::into));
self.debug_assert_args_compatible(def_id, args);
args
}

#[inline]
Expand Down
26 changes: 4 additions & 22 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1624,13 +1624,7 @@ impl<'tcx> Ty<'tcx> {

#[inline]
pub fn new_adt(tcx: TyCtxt<'tcx>, def: AdtDef<'tcx>, args: GenericArgsRef<'tcx>) -> Ty<'tcx> {
debug_assert_eq!(
tcx.generics_of(def.did()).count(),
args.len(),
"wrong number of args for ADT: {:#?} vs {:#?}",
tcx.generics_of(def.did()).params,
args
);
tcx.debug_assert_args_compatible(def.did(), args);
Ty::new(tcx, Adt(def, args))
}

Expand Down Expand Up @@ -1711,11 +1705,7 @@ impl<'tcx> Ty<'tcx> {
def_id: DefId,
closure_args: GenericArgsRef<'tcx>,
) -> Ty<'tcx> {
debug_assert_eq!(
closure_args.len(),
tcx.generics_of(tcx.typeck_root_def_id(def_id)).count() + 3,
"closure constructed with incorrect generic parameters"
);
tcx.debug_assert_args_compatible(def_id, closure_args);
Ty::new(tcx, Closure(def_id, closure_args))
}

Expand All @@ -1725,11 +1715,7 @@ impl<'tcx> Ty<'tcx> {
def_id: DefId,
closure_args: GenericArgsRef<'tcx>,
) -> Ty<'tcx> {
debug_assert_eq!(
closure_args.len(),
tcx.generics_of(tcx.typeck_root_def_id(def_id)).count() + 5,
"closure constructed with incorrect generic parameters"
);
tcx.debug_assert_args_compatible(def_id, closure_args);
Ty::new(tcx, CoroutineClosure(def_id, closure_args))
}

Expand All @@ -1739,11 +1725,7 @@ impl<'tcx> Ty<'tcx> {
def_id: DefId,
coroutine_args: GenericArgsRef<'tcx>,
) -> Ty<'tcx> {
debug_assert_eq!(
coroutine_args.len(),
tcx.generics_of(tcx.typeck_root_def_id(def_id)).count() + 6,
"coroutine constructed with incorrect number of generic parameters"
);
tcx.debug_assert_args_compatible(def_id, coroutine_args);
Ty::new(tcx, Coroutine(def_id, coroutine_args))
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::traits::{check_args_compatible, specialization_graph};
use crate::traits::specialization_graph;

use super::assembly::structural_traits::AsyncCallableRelevantTypes;
use super::assembly::{self, structural_traits, Candidate};
Expand Down Expand Up @@ -247,7 +247,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
assoc_def.defining_node,
);

if !check_args_compatible(tcx, assoc_def.item, args) {
if !tcx.check_args_compatible(assoc_def.item.def_id, args) {
return error_response(
ecx,
"associated item has mismatched generic item arguments",
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ pub use self::specialize::{
pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_normalize::StructurallyNormalizeExt;
pub use self::util::elaborate;
pub use self::util::{
check_args_compatible, supertrait_def_ids, supertraits, transitive_bounds,
transitive_bounds_that_define_assoc_item, SupertraitDefIds,
};
pub use self::util::{expand_trait_aliases, TraitAliasExpander, TraitAliasExpansionInfo};
pub use self::util::{get_vtable_index_of_object_method, impl_item_is_final, upcast_choices};
pub use self::util::{
supertrait_def_ids, supertraits, transitive_bounds, transitive_bounds_that_define_assoc_item,
SupertraitDefIds,
};
pub use self::util::{with_replaced_escaping_bound_vars, BoundVarReplacer, PlaceholderReplacer};

pub use rustc_infer::traits::*;
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use std::ops::ControlFlow;

use super::check_args_compatible;
use super::specialization_graph;
use super::translate_args;
use super::util;
Expand Down Expand Up @@ -2030,7 +2029,7 @@ fn confirm_impl_candidate<'cx, 'tcx>(
} else {
ty.map_bound(|ty| ty.into())
};
if !check_args_compatible(tcx, assoc_ty.item, args) {
if !tcx.check_args_compatible(assoc_ty.item.def_id, args) {
let err = Ty::new_error_with_message(
tcx,
obligation.cause.span,
Expand Down
42 changes: 0 additions & 42 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,48 +344,6 @@ pub enum TupleArgumentsFlag {
No,
}

// Verify that the trait item and its implementation have compatible args lists
pub fn check_args_compatible<'tcx>(
tcx: TyCtxt<'tcx>,
assoc_item: ty::AssocItem,
args: ty::GenericArgsRef<'tcx>,
) -> bool {
fn check_args_compatible_inner<'tcx>(
tcx: TyCtxt<'tcx>,
generics: &'tcx ty::Generics,
args: &'tcx [ty::GenericArg<'tcx>],
) -> bool {
if generics.count() != args.len() {
return false;
}

let (parent_args, own_args) = args.split_at(generics.parent_count);

if let Some(parent) = generics.parent
&& let parent_generics = tcx.generics_of(parent)
&& !check_args_compatible_inner(tcx, parent_generics, parent_args)
{
return false;
}

for (param, arg) in std::iter::zip(&generics.params, own_args) {
match (&param.kind, arg.unpack()) {
(ty::GenericParamDefKind::Type { .. }, ty::GenericArgKind::Type(_))
| (ty::GenericParamDefKind::Lifetime, ty::GenericArgKind::Lifetime(_))
| (ty::GenericParamDefKind::Const { .. }, ty::GenericArgKind::Const(_)) => {}
_ => return false,
}
}

true
}

let generics = tcx.generics_of(assoc_item.def_id);
// Chop off any additional args (RPITIT) args
let args = &args[0..generics.count().min(args.len())];
check_args_compatible_inner(tcx, generics, args)
}

/// Executes `f` on `value` after replacing all escaping bound variables with placeholders
/// and then replaces these placeholders with the original bound variables in the result.
///
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_ty_utils/src/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_middle::ty::util::{CheckRegions, NotUniqueParam};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::{TypeSuperVisitable, TypeVisitable, TypeVisitor};
use rustc_span::Span;
use rustc_trait_selection::traits::check_args_compatible;

use crate::errors::{DuplicateArg, NotParam};

Expand Down Expand Up @@ -250,7 +249,7 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
ty::GenericArgs::identity_for_item(self.tcx, parent),
);

if check_args_compatible(self.tcx, assoc, impl_args) {
if self.tcx.check_args_compatible(assoc.def_id, impl_args) {
self.tcx
.type_of(assoc.def_id)
.instantiate(self.tcx, impl_args)
Expand Down
1 change: 1 addition & 0 deletions tests/ui/impl-trait/in-trait/span-bug-issue-121457.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impl<'a, I: 'a + Iterable> Iterable for &'a I {
//~^ ERROR binding for associated type `Item` references lifetime `'missing`
//~| ERROR binding for associated type `Item` references lifetime `'missing`
//~| ERROR `()` is not an iterator
//~| WARNING impl trait in impl method signature does not match trait method signature
}

fn main() {}
Loading
Loading