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

Make compare_impl_item into a query #133365

Merged
merged 2 commits into from
Dec 1, 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
31 changes: 18 additions & 13 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use tracing::{debug, instrument};
use ty::TypingMode;
use {rustc_attr as attr, rustc_hir as hir};

use super::compare_impl_item::{check_type_bounds, compare_impl_method, compare_impl_ty};
use super::compare_impl_item::check_type_bounds;
use super::*;
use crate::check::intrinsicck::InlineAsmCtxt;

Expand Down Expand Up @@ -1044,18 +1044,23 @@ fn check_impl_items_against_trait<'tcx>(
tcx.dcx().span_delayed_bug(tcx.def_span(impl_item), "missing associated item in trait");
continue;
};
match ty_impl_item.kind {
ty::AssocKind::Const => {
tcx.ensure().compare_impl_const((
impl_item.expect_local(),
ty_impl_item.trait_item_def_id.unwrap(),
));
}
ty::AssocKind::Fn => {
compare_impl_method(tcx, ty_impl_item, ty_trait_item, trait_ref);
}
ty::AssocKind::Type => {
compare_impl_ty(tcx, ty_impl_item, ty_trait_item, trait_ref);

let res = tcx.ensure().compare_impl_item(impl_item.expect_local());

if res.is_ok() {
match ty_impl_item.kind {
ty::AssocKind::Fn => {
compare_impl_item::refine::check_refining_return_position_impl_trait_in_trait(
tcx,
ty_impl_item,
ty_trait_item,
tcx.impl_trait_ref(ty_impl_item.container_id(tcx))
.unwrap()
.instantiate_identity(),
);
}
ty::AssocKind::Const => {}
ty::AssocKind::Type => {}
}
}

Expand Down
68 changes: 36 additions & 32 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,25 @@ use tracing::{debug, instrument};
use super::potentially_plural_count;
use crate::errors::{LifetimesOrBoundsMismatchOnTrait, MethodShouldReturnFuture};

mod refine;
pub(super) mod refine;

/// Call the query `tcx.compare_impl_item()` directly instead.
pub(super) fn compare_impl_item(
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
tcx: TyCtxt<'_>,
impl_item_def_id: LocalDefId,
) -> Result<(), ErrorGuaranteed> {
let impl_item = tcx.associated_item(impl_item_def_id);
let trait_item = tcx.associated_item(impl_item.trait_item_def_id.unwrap());
let impl_trait_ref =
tcx.impl_trait_ref(impl_item.container_id(tcx)).unwrap().instantiate_identity();
debug!(?impl_trait_ref);

match impl_item.kind {
ty::AssocKind::Fn => compare_impl_method(tcx, impl_item, trait_item, impl_trait_ref),
ty::AssocKind::Type => compare_impl_ty(tcx, impl_item, trait_item, impl_trait_ref),
ty::AssocKind::Const => compare_impl_const(tcx, impl_item, trait_item, impl_trait_ref),
}
}

/// Checks that a method from an impl conforms to the signature of
/// the same method as declared in the trait.
Expand All @@ -44,22 +62,15 @@ mod refine;
/// - `trait_m`: the method in the trait
/// - `impl_trait_ref`: the TraitRef corresponding to the trait implementation
#[instrument(level = "debug", skip(tcx))]
pub(super) fn compare_impl_method<'tcx>(
fn compare_impl_method<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m: ty::AssocItem,
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
trait_m: ty::AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
) {
let _: Result<_, ErrorGuaranteed> = try {
check_method_is_structurally_compatible(tcx, impl_m, trait_m, impl_trait_ref, false)?;
compare_method_predicate_entailment(tcx, impl_m, trait_m, impl_trait_ref)?;
refine::check_refining_return_position_impl_trait_in_trait(
tcx,
impl_m,
trait_m,
impl_trait_ref,
);
};
) -> Result<(), ErrorGuaranteed> {
check_method_is_structurally_compatible(tcx, impl_m, trait_m, impl_trait_ref, false)?;
compare_method_predicate_entailment(tcx, impl_m, trait_m, impl_trait_ref)?;
Ok(())
}

/// Checks a bunch of different properties of the impl/trait methods for
Expand Down Expand Up @@ -1721,17 +1732,12 @@ fn compare_generic_param_kinds<'tcx>(
Ok(())
}

/// Use `tcx.compare_impl_const` instead
pub(super) fn compare_impl_const_raw(
tcx: TyCtxt<'_>,
(impl_const_item_def, trait_const_item_def): (LocalDefId, DefId),
fn compare_impl_const<'tcx>(
tcx: TyCtxt<'tcx>,
impl_const_item: ty::AssocItem,
trait_const_item: ty::AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let impl_const_item = tcx.associated_item(impl_const_item_def);
let trait_const_item = tcx.associated_item(trait_const_item_def);
let impl_trait_ref =
tcx.impl_trait_ref(impl_const_item.container_id(tcx)).unwrap().instantiate_identity();
debug!(?impl_trait_ref);

compare_number_of_generics(tcx, impl_const_item, trait_const_item, false)?;
compare_generic_param_kinds(tcx, impl_const_item, trait_const_item, false)?;
check_region_bounds_on_impl_item(tcx, impl_const_item, trait_const_item, false)?;
Expand Down Expand Up @@ -1862,19 +1868,17 @@ fn compare_const_predicate_entailment<'tcx>(
}

#[instrument(level = "debug", skip(tcx))]
pub(super) fn compare_impl_ty<'tcx>(
fn compare_impl_ty<'tcx>(
tcx: TyCtxt<'tcx>,
impl_ty: ty::AssocItem,
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
trait_ty: ty::AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
) {
let _: Result<(), ErrorGuaranteed> = try {
compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)?;
};
) -> Result<(), ErrorGuaranteed> {
compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)
}

/// The equivalent of [compare_method_predicate_entailment], but for associated types
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt;
use rustc_trait_selection::traits::{ObligationCtxt, elaborate, normalize_param_env_or_error};

/// Check that an implementation does not refine an RPITIT from a trait method signature.
pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
pub(crate) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m: ty::AssocItem,
trait_m: ty::AssocItem,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub fn provide(providers: &mut Providers) {
adt_async_destructor,
region_scope_tree,
collect_return_position_impl_trait_in_trait_tys,
compare_impl_const: compare_impl_item::compare_impl_const_raw,
compare_impl_item: compare_impl_item::compare_impl_item,
check_coroutine_obligations: check::check_coroutine_obligations,
..*providers
};
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2311,10 +2311,13 @@ rustc_queries! {
desc { "checking validity requirement for `{}`: {}", key.1.value, key.0 }
}

query compare_impl_const(
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
key: (LocalDefId, DefId)
) -> Result<(), ErrorGuaranteed> {
desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0) }
/// This takes the def-id of an associated item from a impl of a trait,
/// and checks its validity against the trait item it corresponds to.
///
/// Any other def id will ICE.
query compare_impl_item(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
desc { |tcx| "checking assoc item `{}` is compatible with trait definition", tcx.def_path_str(key) }
ensure_forwards_result_if_red
}

query deduced_param_attrs(def_id: DefId) -> &'tcx [ty::DeducedParamAttrs] {
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,15 @@ fn resolve_associated_item<'tcx>(

let args = tcx.erase_regions(args);

// Check if we just resolved an associated `const` declaration from
// a `trait` to an associated `const` definition in an `impl`, where
// the definition in the `impl` has the wrong type (for which an
// error has already been/will be emitted elsewhere).
if leaf_def.item.kind == ty::AssocKind::Const
&& trait_item_id != leaf_def.item.def_id
// We check that the impl item is compatible with the trait item
// because otherwise we may ICE in const eval due to type mismatches,
// signature incompatibilities, etc.
// NOTE: We could also only enforce this in `PostAnalysis`, which
// is what CTFE and MIR inlining would care about anyways.
if trait_item_id != leaf_def.item.def_id
&& let Some(leaf_def_item) = leaf_def.item.def_id.as_local()
{
tcx.compare_impl_const((leaf_def_item, trait_item_id))?;
tcx.ensure().compare_impl_item(leaf_def_item)?;
}

Some(ty::Instance::new(leaf_def.item.def_id, args))
Expand Down
21 changes: 0 additions & 21 deletions tests/crashes/119701.rs

This file was deleted.

23 changes: 0 additions & 23 deletions tests/crashes/121127.rs

This file was deleted.

13 changes: 0 additions & 13 deletions tests/crashes/121411.rs

This file was deleted.

16 changes: 0 additions & 16 deletions tests/crashes/129075.rs

This file was deleted.

21 changes: 0 additions & 21 deletions tests/crashes/129127.rs

This file was deleted.

30 changes: 0 additions & 30 deletions tests/crashes/129214.rs

This file was deleted.

25 changes: 0 additions & 25 deletions tests/crashes/131294-2.rs

This file was deleted.

16 changes: 0 additions & 16 deletions tests/crashes/131294.rs

This file was deleted.

8 changes: 4 additions & 4 deletions tests/ui/const-generics/issues/issue-83765.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ note: ...which requires computing candidate for `<LazyUpdim<'_, T, <T as TensorD
LL | trait TensorDimension {
| ^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires resolving instance `<LazyUpdim<'_, T, <T as TensorDimension>::DIM, DIM> as TensorDimension>::DIM`, completing the cycle
note: cycle used when checking that `<impl at $DIR/issue-83765.rs:56:1: 56:97>` is well-formed
--> $DIR/issue-83765.rs:56:1
note: cycle used when checking assoc item `<impl at $DIR/issue-83765.rs:56:1: 56:97>::bget` is compatible with trait definition
--> $DIR/issue-83765.rs:58:5
|
LL | impl<'a, T: Broadcastable, const DIM: usize> Broadcastable for LazyUpdim<'a, T, { T::DIM }, DIM> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | fn bget(&self, index: [usize; DIM]) -> Option<Self::Element> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0308]: method not compatible with trait
Expand Down
Loading
Loading