about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2022-10-06 16:29:43 +0200
committerGitHub <noreply@github.com>2022-10-06 16:29:43 +0200
commitdd0fa6f871cfbf0d206cd66846bb754cae5c60d3 (patch)
tree531833978577bcf9180d65a4f7ccad153ad02b8d
parent6b6610b8bd5112bbb3bd2b73a97250cb06979505 (diff)
parent25ed5d5db295e1fbf27e511c9159e67c22b08f43 (diff)
downloadrust-dd0fa6f871cfbf0d206cd66846bb754cae5c60d3.tar.gz
rust-dd0fa6f871cfbf0d206cd66846bb754cae5c60d3.zip
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`
-rw-r--r--compiler/rustc_hir_analysis/src/check/check.rs14
-rw-r--r--compiler/rustc_hir_analysis/src/check/compare_method.rs58
-rw-r--r--compiler/rustc_hir_analysis/src/check/mod.rs1
-rw-r--r--compiler/rustc_middle/src/query/mod.rs6
-rw-r--r--compiler/rustc_ty_utils/src/instance.rs36
-rw-r--r--compiler/rustc_ty_utils/src/lib.rs1
-rw-r--r--src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr2
-rw-r--r--src/test/ui/associated-consts/mismatched_impl_ty_1.rs18
-rw-r--r--src/test/ui/associated-consts/mismatched_impl_ty_2.rs11
-rw-r--r--src/test/ui/associated-consts/mismatched_impl_ty_3.rs11
-rw-r--r--src/test/ui/nll/trait-associated-constant.stderr2
11 files changed, 90 insertions, 70 deletions
diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs
index 824144aeac0..a2636f23a4f 100644
--- a/compiler/rustc_hir_analysis/src/check/check.rs
+++ b/compiler/rustc_hir_analysis/src/check/check.rs
@@ -2,7 +2,7 @@ use crate::check::intrinsicck::InlineAsmCtxt;
 
 use super::coercion::CoerceMany;
 use super::compare_method::check_type_bounds;
-use super::compare_method::{compare_const_impl, compare_impl_method, compare_ty_impl};
+use super::compare_method::{compare_impl_method, compare_ty_impl};
 use super::*;
 use rustc_attr as attr;
 use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan};
@@ -1048,14 +1048,10 @@ fn check_impl_items_against_trait<'tcx>(
         let impl_item_full = tcx.hir().impl_item(impl_item.id);
         match impl_item_full.kind {
             hir::ImplItemKind::Const(..) => {
-                // Find associated const definition.
-                compare_const_impl(
-                    tcx,
-                    &ty_impl_item,
-                    impl_item.span,
-                    &ty_trait_item,
-                    impl_trait_ref,
-                );
+                let _ = tcx.compare_assoc_const_impl_item_with_trait_item((
+                    impl_item.id.def_id.def_id,
+                    ty_impl_item.trait_item_def_id.unwrap(),
+                ));
             }
             hir::ImplItemKind::Fn(..) => {
                 let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs
index ae98a8f6209..d006948c587 100644
--- a/compiler/rustc_hir_analysis/src/check/compare_method.rs
+++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs
@@ -1,6 +1,6 @@
 use super::potentially_plural_count;
 use crate::errors::LifetimesOrBoundsMismatchOnTrait;
-use hir::def_id::DefId;
+use hir::def_id::{DefId, LocalDefId};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
 use rustc_hir as hir;
@@ -1300,17 +1300,20 @@ fn compare_generic_param_kinds<'tcx>(
     Ok(())
 }
 
-pub(crate) fn compare_const_impl<'tcx>(
+/// Use `tcx.compare_assoc_const_impl_item_with_trait_item` instead
+pub(crate) fn raw_compare_const_impl<'tcx>(
     tcx: TyCtxt<'tcx>,
-    impl_c: &ty::AssocItem,
-    impl_c_span: Span,
-    trait_c: &ty::AssocItem,
-    impl_trait_ref: ty::TraitRef<'tcx>,
-) {
+    (impl_const_item_def, trait_const_item_def): (LocalDefId, DefId),
+) -> 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();
     debug!("compare_const_impl(impl_trait_ref={:?})", impl_trait_ref);
 
+    let impl_c_span = tcx.def_span(impl_const_item_def.to_def_id());
+
     tcx.infer_ctxt().enter(|infcx| {
-        let param_env = tcx.param_env(impl_c.def_id);
+        let param_env = tcx.param_env(impl_const_item_def.to_def_id());
         let ocx = ObligationCtxt::new(&infcx);
 
         // The below is for the most part highly similar to the procedure
@@ -1322,18 +1325,18 @@ pub(crate) fn compare_const_impl<'tcx>(
 
         // Create a parameter environment that represents the implementation's
         // method.
-        let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_c.def_id.expect_local());
+        let impl_c_hir_id = tcx.hir().local_def_id_to_hir_id(impl_const_item_def);
 
         // Compute placeholder form of impl and trait const tys.
-        let impl_ty = tcx.type_of(impl_c.def_id);
-        let trait_ty = tcx.bound_type_of(trait_c.def_id).subst(tcx, trait_to_impl_substs);
+        let impl_ty = tcx.type_of(impl_const_item_def.to_def_id());
+        let trait_ty = tcx.bound_type_of(trait_const_item_def).subst(tcx, trait_to_impl_substs);
         let mut cause = ObligationCause::new(
             impl_c_span,
             impl_c_hir_id,
             ObligationCauseCode::CompareImplItemObligation {
-                impl_item_def_id: impl_c.def_id.expect_local(),
-                trait_item_def_id: trait_c.def_id,
-                kind: impl_c.kind,
+                impl_item_def_id: impl_const_item_def,
+                trait_item_def_id: trait_const_item_def,
+                kind: impl_const_item.kind,
             },
         );
 
@@ -1358,9 +1361,9 @@ pub(crate) fn compare_const_impl<'tcx>(
             );
 
             // Locate the Span containing just the type of the offending impl
-            match tcx.hir().expect_impl_item(impl_c.def_id.expect_local()).kind {
+            match tcx.hir().expect_impl_item(impl_const_item_def).kind {
                 ImplItemKind::Const(ref ty, _) => cause.span = ty.span,
-                _ => bug!("{:?} is not a impl const", impl_c),
+                _ => bug!("{:?} is not a impl const", impl_const_item),
             }
 
             let mut diag = struct_span_err!(
@@ -1368,14 +1371,14 @@ pub(crate) fn compare_const_impl<'tcx>(
                 cause.span,
                 E0326,
                 "implemented const `{}` has an incompatible type for trait",
-                trait_c.name
+                trait_const_item.name
             );
 
-            let trait_c_span = trait_c.def_id.as_local().map(|trait_c_def_id| {
+            let trait_c_span = trait_const_item_def.as_local().map(|trait_c_def_id| {
                 // Add a label to the Span containing just the type of the const
                 match tcx.hir().expect_trait_item(trait_c_def_id).kind {
                     TraitItemKind::Const(ref ty, _) => ty.span,
-                    _ => bug!("{:?} is not a trait const", trait_c),
+                    _ => bug!("{:?} is not a trait const", trait_const_item),
                 }
             });
 
@@ -1391,23 +1394,22 @@ pub(crate) fn compare_const_impl<'tcx>(
                 false,
                 false,
             );
-            diag.emit();
-        }
+            return Err(diag.emit());
+        };
 
         // Check that all obligations are satisfied by the implementation's
         // version.
         let errors = ocx.select_all_or_error();
         if !errors.is_empty() {
-            infcx.report_fulfillment_errors(&errors, None, false);
-            return;
+            return Err(infcx.report_fulfillment_errors(&errors, None, false));
         }
 
+        // FIXME return `ErrorReported` if region obligations error?
         let outlives_environment = OutlivesEnvironment::new(param_env);
-        infcx.check_region_obligations_and_report_errors(
-            impl_c.def_id.expect_local(),
-            &outlives_environment,
-        );
-    });
+        infcx
+            .check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment);
+        Ok(())
+    })
 }
 
 pub(crate) fn compare_ty_impl<'tcx>(
diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs
index 593a9776bde..04e8c9c22d1 100644
--- a/compiler/rustc_hir_analysis/src/check/mod.rs
+++ b/compiler/rustc_hir_analysis/src/check/mod.rs
@@ -251,6 +251,7 @@ pub fn provide(providers: &mut Providers) {
         check_mod_item_types,
         region_scope_tree,
         collect_trait_impl_trait_tys,
+        compare_assoc_const_impl_item_with_trait_item: compare_method::raw_compare_const_impl,
         ..*providers
     };
 }
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
index cf5b365b27c..334eb953cbc 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -2105,4 +2105,10 @@ rustc_queries! {
     query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
         desc { "checking to see if {:?} permits being left zeroed", key.ty }
     }
+
+    query compare_assoc_const_impl_item_with_trait_item(
+        key: (LocalDefId, DefId)
+    ) -> Result<(), ErrorGuaranteed> {
+        desc { |tcx| "checking assoc const `{}` has the same type as trait item", tcx.def_path_str(key.0.to_def_id()) }
+    }
 }
diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs
index 81ca8b646ff..fe6ebd4b935 100644
--- a/compiler/rustc_ty_utils/src/instance.rs
+++ b/compiler/rustc_ty_utils/src/instance.rs
@@ -186,40 +186,14 @@ fn resolve_associated_item<'tcx>(
             // 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).
-            //
-            // NB: this may be expensive, we try to skip it in all the cases where
-            // we know the error would've been caught (e.g. in an upstream crate).
-            //
-            // A better approach might be to just introduce a query (returning
-            // `Result<(), ErrorGuaranteed>`) for the check that `rustc_hir_analysis`
-            // performs (i.e. that the definition's type in the `impl` matches
-            // the declaration in the `trait`), so that we can cheaply check
-            // here if it failed, instead of approximating it.
             if leaf_def.item.kind == ty::AssocKind::Const
                 && trait_item_id != leaf_def.item.def_id
-                && leaf_def.item.def_id.is_local()
+                && let Some(leaf_def_item) = leaf_def.item.def_id.as_local()
             {
-                let normalized_type_of = |def_id, substs| {
-                    tcx.subst_and_normalize_erasing_regions(substs, param_env, tcx.type_of(def_id))
-                };
-
-                let original_ty = normalized_type_of(trait_item_id, rcvr_substs);
-                let resolved_ty = normalized_type_of(leaf_def.item.def_id, substs);
-
-                if original_ty != resolved_ty {
-                    let msg = format!(
-                        "Instance::resolve: inconsistent associated `const` type: \
-                         was `{}: {}` but resolved to `{}: {}`",
-                        tcx.def_path_str_with_substs(trait_item_id, rcvr_substs),
-                        original_ty,
-                        tcx.def_path_str_with_substs(leaf_def.item.def_id, substs),
-                        resolved_ty,
-                    );
-                    let span = tcx.def_span(leaf_def.item.def_id);
-                    let reported = tcx.sess.delay_span_bug(span, &msg);
-
-                    return Err(reported);
-                }
+                tcx.compare_assoc_const_impl_item_with_trait_item((
+                    leaf_def_item,
+                    trait_item_id,
+                ))?;
             }
 
             Some(ty::Instance::new(leaf_def.item.def_id, substs))
diff --git a/compiler/rustc_ty_utils/src/lib.rs b/compiler/rustc_ty_utils/src/lib.rs
index f97fc4c199d..0ddc154fbeb 100644
--- a/compiler/rustc_ty_utils/src/lib.rs
+++ b/compiler/rustc_ty_utils/src/lib.rs
@@ -5,6 +5,7 @@
 //! This API is completely unstable and subject to change.
 
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
+#![feature(let_chains)]
 #![feature(control_flow_enum)]
 #![feature(never_type)]
 #![feature(box_patterns)]
diff --git a/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr b/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr
index de1d9589e99..742b81535df 100644
--- a/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr
+++ b/src/test/ui/associated-consts/associated-const-impl-wrong-lifetime.stderr
@@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait
   --> $DIR/associated-const-impl-wrong-lifetime.rs:7:5
    |
 LL |     const NAME: &'a str = "unit";
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
+   |     ^^^^^^^^^^^^^^^^^^^ lifetime mismatch
    |
    = note: expected reference `&'static str`
               found reference `&'a str`
diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_1.rs b/src/test/ui/associated-consts/mismatched_impl_ty_1.rs
new file mode 100644
index 00000000000..4dc6c2e47a9
--- /dev/null
+++ b/src/test/ui/associated-consts/mismatched_impl_ty_1.rs
@@ -0,0 +1,18 @@
+// run-pass
+#![feature(generic_const_exprs)]
+#![allow(incomplete_features)]
+
+trait MyTrait {
+    type ArrayType;
+    const SIZE: usize;
+    const ARRAY: Self::ArrayType;
+}
+impl MyTrait for () {
+    type ArrayType = [u8; Self::SIZE];
+    const SIZE: usize = 4;
+    const ARRAY: [u8; Self::SIZE] = [1, 2, 3, 4];
+}
+
+fn main() {
+    let _ = <() as MyTrait>::ARRAY;
+}
diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_2.rs b/src/test/ui/associated-consts/mismatched_impl_ty_2.rs
new file mode 100644
index 00000000000..539becfdc7c
--- /dev/null
+++ b/src/test/ui/associated-consts/mismatched_impl_ty_2.rs
@@ -0,0 +1,11 @@
+// run-pass
+trait Trait {
+    const ASSOC: fn(&'static u32);
+}
+impl Trait for () {
+    const ASSOC: for<'a> fn(&'a u32) = |_| ();
+}
+
+fn main() {
+    let _ = <() as Trait>::ASSOC;
+}
diff --git a/src/test/ui/associated-consts/mismatched_impl_ty_3.rs b/src/test/ui/associated-consts/mismatched_impl_ty_3.rs
new file mode 100644
index 00000000000..17bcc8fe576
--- /dev/null
+++ b/src/test/ui/associated-consts/mismatched_impl_ty_3.rs
@@ -0,0 +1,11 @@
+// run-pass
+trait Trait {
+    const ASSOC: for<'a, 'b> fn(&'a u32, &'b u32);
+}
+impl Trait for () {
+    const ASSOC: for<'a> fn(&'a u32, &'a u32) = |_, _| ();
+}
+
+fn main() {
+    let _ = <() as Trait>::ASSOC;
+}
diff --git a/src/test/ui/nll/trait-associated-constant.stderr b/src/test/ui/nll/trait-associated-constant.stderr
index ae0ffd904e7..cf1c52ba7cc 100644
--- a/src/test/ui/nll/trait-associated-constant.stderr
+++ b/src/test/ui/nll/trait-associated-constant.stderr
@@ -2,7 +2,7 @@ error[E0308]: const not compatible with trait
   --> $DIR/trait-associated-constant.rs:21:5
    |
 LL |     const AC: Option<&'c str> = None;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
    |
    = note: expected enum `Option<&'b str>`
               found enum `Option<&'c str>`