Skip to content

Commit dd0fa6f

Browse files
authored
Rollup merge of #98496 - BoxyUwU:instancers_bad_equality, r=lcnr
make `compare_const_impl` a query and use it in `instance.rs` Fixes #88365 the bug in #88365 was caused by some `instance.rs` code using the `PartialEq` impl on `Ty` to check that the type of the associated const in an impl is the same as the type of the associated const in the trait definition. This was wrong for two reasons: - the check typeck does is that the impl type is a subtype of the trait definition's type (see `mismatched_impl_ty_2.rs` which [was ICEing](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f6d60ebe6745011f0d52ab2bc712025d) before this PR on stable) - it assumes that if two types are equal then the `PartialEq` impl will reflect that which isnt true for higher ranked types or type level constants when `feature(generic_const_exprs)` is enabled (see `mismatched_impl_ty_3.rs` for higher ranked types which was [ICEing on stable](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d7af131a655ed515b035624626c62c71)) r? `@lcnr`
2 parents 6b6610b + 25ed5d5 commit dd0fa6f

File tree

11 files changed

+90
-70
lines changed

11 files changed

+90
-70
lines changed

compiler/rustc_hir_analysis/src/check/check.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::check::intrinsicck::InlineAsmCtxt;
22

33
use super::coercion::CoerceMany;
44
use super::compare_method::check_type_bounds;
5-
use super::compare_method::{compare_const_impl, compare_impl_method, compare_ty_impl};
5+
use super::compare_method::{compare_impl_method, compare_ty_impl};
66
use super::*;
77
use rustc_attr as attr;
88
use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan};
@@ -1048,14 +1048,10 @@ fn check_impl_items_against_trait<'tcx>(
10481048
let impl_item_full = tcx.hir().impl_item(impl_item.id);
10491049
match impl_item_full.kind {
10501050
hir::ImplItemKind::Const(..) => {
1051-
// Find associated const definition.
1052-
compare_const_impl(
1053-
tcx,
1054-
&ty_impl_item,
1055-
impl_item.span,
1056-
&ty_trait_item,
1057-
impl_trait_ref,
1058-
);
1051+
let _ = tcx.compare_assoc_const_impl_item_with_trait_item((
1052+
impl_item.id.def_id.def_id,
1053+
ty_impl_item.trait_item_def_id.unwrap(),
1054+
));
10591055
}
10601056
hir::ImplItemKind::Fn(..) => {
10611057
let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);

compiler/rustc_hir_analysis/src/check/compare_method.rs

+30-28
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::potentially_plural_count;
22
use crate::errors::LifetimesOrBoundsMismatchOnTrait;
3-
use hir::def_id::DefId;
3+
use hir::def_id::{DefId, LocalDefId};
44
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
55
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
66
use rustc_hir as hir;
@@ -1300,17 +1300,20 @@ fn compare_generic_param_kinds<'tcx>(
13001300
Ok(())
13011301
}
13021302

1303-
pub(crate) fn compare_const_impl<'tcx>(
1303+
/// Use `tcx.compare_assoc_const_impl_item_with_trait_item` instead
1304+
pub(crate) fn raw_compare_const_impl<'tcx>(
13041305
tcx: TyCtxt<'tcx>,
1305-
impl_c: &ty::AssocItem,
1306-
impl_c_span: Span,
1307-
trait_c: &ty::AssocItem,
1308-
impl_trait_ref: ty::TraitRef<'tcx>,
1309-
) {
1306+
(impl_const_item_def, trait_const_item_def): (LocalDefId, DefId),
1307+
) -> Result<(), ErrorGuaranteed> {
1308+
let impl_const_item = tcx.associated_item(impl_const_item_def);
1309+
let trait_const_item = tcx.associated_item(trait_const_item_def);
1310+
let impl_trait_ref = tcx.impl_trait_ref(impl_const_item.container_id(tcx)).unwrap();
13101311
debug!("compare_const_impl(impl_trait_ref={:?})", impl_trait_ref);
13111312

1313+
let impl_c_span = tcx.def_span(impl_const_item_def.to_def_id());
1314+
13121315
tcx.infer_ctxt().enter(|infcx| {
1313-
let param_env = tcx.param_env(impl_c.def_id);
1316+
let param_env = tcx.param_env(impl_const_item_def.to_def_id());
13141317
let ocx = ObligationCtxt::new(&infcx);
13151318

13161319
// The below is for the most part highly similar to the procedure
@@ -1322,18 +1325,18 @@ pub(crate) fn compare_const_impl<'tcx>(
13221325

13231326
// Create a parameter environment that represents the implementation's
13241327
// method.
1325-
let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_c.def_id.expect_local());
1328+
let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_const_item_def);
13261329

13271330
// Compute placeholder form of impl and trait const tys.
1328-
let impl_ty = tcx.type_of(impl_c.def_id);
1329-
let trait_ty = tcx.bound_type_of(trait_c.def_id).subst(tcx, trait_to_impl_substs);
1331+
let impl_ty = tcx.type_of(impl_const_item_def.to_def_id());
1332+
let trait_ty = tcx.bound_type_of(trait_const_item_def).subst(tcx, trait_to_impl_substs);
13301333
let mut cause = ObligationCause::new(
13311334
impl_c_span,
13321335
impl_c_hir_id,
13331336
ObligationCauseCode::CompareImplItemObligation {
1334-
impl_item_def_id: impl_c.def_id.expect_local(),
1335-
trait_item_def_id: trait_c.def_id,
1336-
kind: impl_c.kind,
1337+
impl_item_def_id: impl_const_item_def,
1338+
trait_item_def_id: trait_const_item_def,
1339+
kind: impl_const_item.kind,
13371340
},
13381341
);
13391342

@@ -1358,24 +1361,24 @@ pub(crate) fn compare_const_impl<'tcx>(
13581361
);
13591362

13601363
// Locate the Span containing just the type of the offending impl
1361-
match tcx.hir().expect_impl_item(impl_c.def_id.expect_local()).kind {
1364+
match tcx.hir().expect_impl_item(impl_const_item_def).kind {
13621365
ImplItemKind::Const(ref ty, _) => cause.span = ty.span,
1363-
_ => bug!("{:?} is not a impl const", impl_c),
1366+
_ => bug!("{:?} is not a impl const", impl_const_item),
13641367
}
13651368

13661369
let mut diag = struct_span_err!(
13671370
tcx.sess,
13681371
cause.span,
13691372
E0326,
13701373
"implemented const `{}` has an incompatible type for trait",
1371-
trait_c.name
1374+
trait_const_item.name
13721375
);
13731376

1374-
let trait_c_span = trait_c.def_id.as_local().map(|trait_c_def_id| {
1377+
let trait_c_span = trait_const_item_def.as_local().map(|trait_c_def_id| {
13751378
// Add a label to the Span containing just the type of the const
13761379
match tcx.hir().expect_trait_item(trait_c_def_id).kind {
13771380
TraitItemKind::Const(ref ty, _) => ty.span,
1378-
_ => bug!("{:?} is not a trait const", trait_c),
1381+
_ => bug!("{:?} is not a trait const", trait_const_item),
13791382
}
13801383
});
13811384

@@ -1391,23 +1394,22 @@ pub(crate) fn compare_const_impl<'tcx>(
13911394
false,
13921395
false,
13931396
);
1394-
diag.emit();
1395-
}
1397+
return Err(diag.emit());
1398+
};
13961399

13971400
// Check that all obligations are satisfied by the implementation's
13981401
// version.
13991402
let errors = ocx.select_all_or_error();
14001403
if !errors.is_empty() {
1401-
infcx.report_fulfillment_errors(&errors, None, false);
1402-
return;
1404+
return Err(infcx.report_fulfillment_errors(&errors, None, false));
14031405
}
14041406

1407+
// FIXME return `ErrorReported` if region obligations error?
14051408
let outlives_environment = OutlivesEnvironment::new(param_env);
1406-
infcx.check_region_obligations_and_report_errors(
1407-
impl_c.def_id.expect_local(),
1408-
&outlives_environment,
1409-
);
1410-
});
1409+
infcx
1410+
.check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment);
1411+
Ok(())
1412+
})
14111413
}
14121414

14131415
pub(crate) fn compare_ty_impl<'tcx>(

compiler/rustc_hir_analysis/src/check/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ pub fn provide(providers: &mut Providers) {
251251
check_mod_item_types,
252252
region_scope_tree,
253253
collect_trait_impl_trait_tys,
254+
compare_assoc_const_impl_item_with_trait_item: compare_method::raw_compare_const_impl,
254255
..*providers
255256
};
256257
}

compiler/rustc_middle/src/query/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -2105,4 +2105,10 @@ rustc_queries! {
21052105
query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
21062106
desc { "checking to see if {:?} permits being left zeroed", key.ty }
21072107
}
2108+
2109+
query compare_assoc_const_impl_item_with_trait_item(
2110+
key: (LocalDefId, DefId)
2111+
) -> Result<(), ErrorGuaranteed> {
2112+
desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0.to_def_id()) }
2113+
}
21082114
}

compiler/rustc_ty_utils/src/instance.rs

+5-31
Original file line numberDiff line numberDiff line change
@@ -186,40 +186,14 @@ fn resolve_associated_item<'tcx>(
186186
// a `trait` to an associated `const` definition in an `impl`, where
187187
// the definition in the `impl` has the wrong type (for which an
188188
// error has already been/will be emitted elsewhere).
189-
//
190-
// NB: this may be expensive, we try to skip it in all the cases where
191-
// we know the error would've been caught (e.g. in an upstream crate).
192-
//
193-
// A better approach might be to just introduce a query (returning
194-
// `Result<(), ErrorGuaranteed>`) for the check that `rustc_hir_analysis`
195-
// performs (i.e. that the definition's type in the `impl` matches
196-
// the declaration in the `trait`), so that we can cheaply check
197-
// here if it failed, instead of approximating it.
198189
if leaf_def.item.kind == ty::AssocKind::Const
199190
&& trait_item_id != leaf_def.item.def_id
200-
&& leaf_def.item.def_id.is_local()
191+
&& let Some(leaf_def_item) = leaf_def.item.def_id.as_local()
201192
{
202-
let normalized_type_of = |def_id, substs| {
203-
tcx.subst_and_normalize_erasing_regions(substs, param_env, tcx.type_of(def_id))
204-
};
205-
206-
let original_ty = normalized_type_of(trait_item_id, rcvr_substs);
207-
let resolved_ty = normalized_type_of(leaf_def.item.def_id, substs);
208-
209-
if original_ty != resolved_ty {
210-
let msg = format!(
211-
"Instance::resolve: inconsistent associated `const` type: \
212-
was `{}: {}` but resolved to `{}: {}`",
213-
tcx.def_path_str_with_substs(trait_item_id, rcvr_substs),
214-
original_ty,
215-
tcx.def_path_str_with_substs(leaf_def.item.def_id, substs),
216-
resolved_ty,
217-
);
218-
let span = tcx.def_span(leaf_def.item.def_id);
219-
let reported = tcx.sess.delay_span_bug(span, &msg);
220-
221-
return Err(reported);
222-
}
193+
tcx.compare_assoc_const_impl_item_with_trait_item((
194+
leaf_def_item,
195+
trait_item_id,
196+
))?;
223197
}
224198

225199
Some(ty::Instance::new(leaf_def.item.def_id, substs))

compiler/rustc_ty_utils/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//! This API is completely unstable and subject to change.
66
77
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
8+
#![feature(let_chains)]
89
#![feature(control_flow_enum)]
910
#![feature(never_type)]
1011
#![feature(box_patterns)]

src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait
22
--> $DIR/associated-const-impl-wrong-lifetime.rs:7:5
33
|
44
LL | const NAME: &'a str = "unit";
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
5+
| ^^^^^^^^^^^^^^^^^^^ lifetime mismatch
66
|
77
= note: expected reference `&'static str`
88
found reference `&'a str`
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// run-pass
2+
#![feature(generic_const_exprs)]
3+
#![allow(incomplete_features)]
4+
5+
trait MyTrait {
6+
type ArrayType;
7+
const SIZE: usize;
8+
const ARRAY: Self::ArrayType;
9+
}
10+
impl MyTrait for () {
11+
type ArrayType = [u8; Self::SIZE];
12+
const SIZE: usize = 4;
13+
const ARRAY: [u8; Self::SIZE] = [1, 2, 3, 4];
14+
}
15+
16+
fn main() {
17+
let _ = <() as MyTrait>::ARRAY;
18+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// run-pass
2+
trait Trait {
3+
const ASSOC: fn(&'static u32);
4+
}
5+
impl Trait for () {
6+
const ASSOC: for<'a> fn(&'a u32) = |_| ();
7+
}
8+
9+
fn main() {
10+
let _ = <() as Trait>::ASSOC;
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// run-pass
2+
trait Trait {
3+
const ASSOC: for<'a, 'b> fn(&'a u32, &'b u32);
4+
}
5+
impl Trait for () {
6+
const ASSOC: for<'a> fn(&'a u32, &'a u32) = |_, _| ();
7+
}
8+
9+
fn main() {
10+
let _ = <() as Trait>::ASSOC;
11+
}

src/test/ui/nll/trait-associated-constant.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait
22
--> $DIR/trait-associated-constant.rs:21:5
33
|
44
LL | const AC: Option<&'c str> = None;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
66
|
77
= note: expected enum `Option<&'b str>`
88
found enum `Option<&'c str>`

0 commit comments

Comments
 (0)