diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index d8e81827a3be1..17a4f2d76b60f 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -339,8 +339,8 @@ fn check_opaque_type_well_formed<'tcx>( // version. let errors = ocx.select_all_or_error(); - // This is still required for many(half of the tests in ui/type-alias-impl-trait) - // tests to pass + // This is fishy, but we check it again in `check_opaque_meets_bounds`. + // Remove once we can prepopulate with known hidden types. let _ = infcx.take_opaque_types(); if errors.is_empty() { diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index d6a2ffb75111a..88d4f5e715c97 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -56,8 +56,13 @@ pub fn is_subtype<'tcx>( // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` // we would get unification errors because we're unable to look into opaque types, // even if they're constrained in our current function. - // - // It seems very unlikely that this hides any bugs. - let _ = infcx.take_opaque_types(); + for (key, ty) in infcx.take_opaque_types() { + span_bug!( + ty.hidden_type.span, + "{}, {}", + tcx.type_of(key.def_id).instantiate(tcx, key.args), + ty.hidden_type.ty + ); + } errors.is_empty() } diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index e0698a72335f3..a876f169c3aa6 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -19,11 +19,13 @@ use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS; use rustc_middle::hir::nested_filter; use rustc_middle::middle::stability::EvalResult; use rustc_middle::traits::DefiningAnchor; +use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; use rustc_middle::ty::util::{Discr, IntTypeExt}; use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::{ - self, AdtDef, ParamEnv, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, + self, AdtDef, ParamEnv, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, + TypeVisitableExt, }; use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS}; use rustc_span::symbol::sym; @@ -34,6 +36,7 @@ use rustc_trait_selection::traits::error_reporting::on_unimplemented::OnUnimplem use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCtxt, TraitEngine, TraitEngineExt as _}; +use rustc_type_ir::fold::TypeFoldable; use std::ops::ControlFlow; @@ -437,7 +440,7 @@ fn check_opaque_meets_bounds<'tcx>( // hidden type is well formed even without those bounds. let predicate = ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(hidden_ty.into()))); - ocx.register_obligation(Obligation::new(tcx, misc_cause, param_env, predicate)); + ocx.register_obligation(Obligation::new(tcx, misc_cause.clone(), param_env, predicate)); // Check that all obligations are satisfied by the implementation's // version. @@ -464,11 +467,179 @@ fn check_opaque_meets_bounds<'tcx>( ocx.resolve_regions_and_report_errors(defining_use_anchor, &outlives_env)?; } } - // Clean up after ourselves - let _ = infcx.take_opaque_types(); + // Check that any hidden types found during wf checking match the hidden types that `type_of` sees. + for (key, mut ty) in infcx.take_opaque_types() { + ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty); + sanity_check_found_hidden_type(tcx, key, ty.hidden_type, defining_use_anchor, origin)?; + } Ok(()) } +fn sanity_check_found_hidden_type<'tcx>( + tcx: TyCtxt<'tcx>, + key: ty::OpaqueTypeKey<'tcx>, + mut ty: ty::OpaqueHiddenType<'tcx>, + defining_use_anchor: LocalDefId, + origin: &hir::OpaqueTyOrigin, +) -> Result<(), ErrorGuaranteed> { + if ty.ty.is_ty_var() { + // Nothing was actually constrained. + return Ok(()); + } + if let ty::Alias(ty::Opaque, alias) = ty.ty.kind() { + if alias.def_id == key.def_id.to_def_id() && alias.args == key.args { + // Nothing was actually constrained, this is an opaque usage that was + // only discovered to be opaque after inference vars resolved. + return Ok(()); + } + } + // Closures frequently end up containing erased lifetimes in their final representation. + // These correspond to lifetime variables that never got resolved, so we patch this up here. + ty.ty = ty.ty.fold_with(&mut BottomUpFolder { + tcx, + ty_op: |t| t, + ct_op: |c| c, + lt_op: |l| match l.kind() { + RegionKind::ReVar(_) => tcx.lifetimes.re_erased, + _ => l, + }, + }); + // Get the hidden type. + let mut hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args); + if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin { + if hidden_ty != ty.ty { + hidden_ty = find_and_apply_rpit_args( + tcx, + hidden_ty, + defining_use_anchor.to_def_id(), + key.def_id.to_def_id(), + )?; + } + } + + // If the hidden types differ, emit a type mismatch diagnostic. + if hidden_ty == ty.ty { + Ok(()) + } else { + let span = tcx.def_span(key.def_id); + let other = ty::OpaqueHiddenType { ty: hidden_ty, span }; + Err(ty.report_mismatch(&other, key.def_id, tcx).emit()) + } +} + +/// In case it is in a nested opaque type, find that opaque type's +/// usage in the function signature and use the generic arguments from the usage site. +/// We need to do because RPITs ignore the lifetimes of the function, +/// as they have their own copies of all the lifetimes they capture. +/// So the only way to get the lifetimes represented in terms of the function, +/// is to look how they are used in the function signature (or do some other fancy +/// recording of this mapping at ast -> hir lowering time). +/// +/// As an example: +/// ```text +/// trait Id { +/// type Assoc; +/// } +/// impl<'a> Id for &'a () { +/// type Assoc = &'a (); +/// } +/// fn func<'a>(x: &'a ()) -> impl Id { x } +/// // desugared to +/// fn func<'a>(x: &'a () -> Outer<'a> where as Id>::Assoc = Inner<'a> { +/// // Note that in contrast to other nested items, RPIT type aliases can +/// // access their parents' generics. +/// +/// // hidden type is `&'aDupOuter ()` +/// // During wfcheck the hidden type of `Inner<'aDupOuter>` is `&'a ()`, but +/// // `typeof(Inner<'aDupOuter>) = &'aDupOuter ()`. +/// // So we walk the signature of `func` to find the use of `Inner<'a>` +/// // and then use that to replace the lifetimes in the hidden type, obtaining +/// // `&'a ()`. +/// type Outer<'aDupOuter> = impl Id>; +/// +/// // hidden type is `&'aDupInner ()` +/// type Inner<'aDupInner> = impl Sized + 'aDupInner; +/// +/// x +/// } +/// ``` +fn find_and_apply_rpit_args<'tcx>( + tcx: TyCtxt<'tcx>, + mut hidden_ty: Ty<'tcx>, + function: DefId, + opaque: DefId, +) -> Result, ErrorGuaranteed> { + // Find use of the RPIT in the function signature and thus find the right args to + // convert it into the parameter space of the function signature. This is needed, + // because that's what `type_of` returns, against which we compare later. + let ret = tcx.fn_sig(function).instantiate_identity().output(); + struct Visitor<'tcx> { + tcx: TyCtxt<'tcx>, + opaque: DefId, + function: DefId, + seen: FxHashSet, + } + impl<'tcx> ty::TypeVisitor> for Visitor<'tcx> { + type BreakTy = GenericArgsRef<'tcx>; + + #[instrument(level = "trace", skip(self), ret)] + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + trace!("{:#?}", t.kind()); + match t.kind() { + ty::Alias(ty::Opaque, alias) => { + trace!(?alias.def_id); + if alias.def_id == self.opaque { + return ControlFlow::Break(alias.args); + } else if self.seen.insert(alias.def_id) { + for clause in self + .tcx + .explicit_item_bounds(alias.def_id) + .iter_instantiated_copied(self.tcx, alias.args) + { + trace!(?clause); + clause.visit_with(self)?; + } + } + } + ty::Alias(ty::Projection, alias) => { + if self.tcx.is_impl_trait_in_trait(alias.def_id) + && self.tcx.impl_trait_in_trait_parent_fn(alias.def_id) == self.function + { + // If we're lowering to associated item, install the opaque type which is just + // the `type_of` of the trait's associated item. If we're using the old lowering + // strategy, then just reinterpret the associated type like an opaque :^) + self.tcx + .type_of(alias.def_id) + .instantiate(self.tcx, alias.args) + .visit_with(self)?; + } + } + ty::Alias(ty::Weak, alias) => { + self.tcx + .type_of(alias.def_id) + .instantiate(self.tcx, alias.args) + .visit_with(self)?; + } + _ => (), + } + + t.super_visit_with(self) + } + } + if let ControlFlow::Break(args) = + ret.visit_with(&mut Visitor { tcx, function, opaque, seen: Default::default() }) + { + trace!(?args); + trace!("expected: {hidden_ty:#?}"); + hidden_ty = ty::EarlyBinder::bind(hidden_ty).instantiate(tcx, args); + trace!("expected: {hidden_ty:#?}"); + } else { + tcx.sess + .delay_span_bug(tcx.def_span(function), format!("{ret:?} does not contain {opaque:?}")); + } + Ok(hidden_ty) +} + fn is_enum_of_nonnullable_ptr<'tcx>( tcx: TyCtxt<'tcx>, adt_def: AdtDef<'tcx>, diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index b04d77f0a1a3a..f48a992d32758 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -272,6 +272,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { // assertions against dropping an `InferCtxt` without taking opaques. // FIXME: Once we remove support for the old impl we can remove this. if input.anchor != DefiningAnchor::Error { + // This seems ok, but fragile. let _ = infcx.take_opaque_types(); } diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 18cee7d3d5c2c..699559e4ba9c7 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. -const ISSUES_ENTRY_LIMIT: usize = 1894; +const ISSUES_ENTRY_LIMIT: usize = 1893; const ROOT_ENTRY_LIMIT: usize = 870; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ diff --git a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs new file mode 100644 index 0000000000000..12ce6b14e31da --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.rs @@ -0,0 +1,57 @@ +//! This test checks that we don't lose hidden types +//! for *other* opaque types that we register and use +//! to prove bounds while checking that a hidden type +//! satisfies its opaque type's bounds. + +#![feature(trivial_bounds, type_alias_impl_trait)] +#![allow(trivial_bounds)] + +mod sus { + use super::*; + pub type Sep = impl Sized + std::fmt::Display; + //~^ ERROR: concrete type differs from previous defining opaque type use + pub fn mk_sep() -> Sep { + String::from("hello") + } + + pub trait Proj { + type Assoc; + } + impl Proj for () { + type Assoc = sus::Sep; + } + + pub struct Bar { + pub inner: ::Assoc, + pub _marker: T, + } + impl Clone for Bar { + fn clone(&self) -> Self { + todo!() + } + } + impl + Copy> Copy for Bar {} + // This allows producing `Tait`s via `From`, even though + // `define_tait` is not actually callable, and thus assumed + // `Bar<()>: Copy` even though it isn't. + pub type Tait = impl Copy + From> + Into>; + pub fn define_tait() -> Tait + where + // this proves `Bar<()>: Copy`, but `define_tait` is + // now uncallable + (): Proj, + { + Bar { inner: 1i32, _marker: () } + } +} + +fn copy_tait(x: sus::Tait) -> (sus::Tait, sus::Tait) { + (x, x) +} + +fn main() { + let bar = sus::Bar { inner: sus::mk_sep(), _marker: () }; + let (y, z) = copy_tait(bar.into()); // copy a string + drop(y.into()); // drop one instance + println!("{}", z.into().inner); // print the other +} diff --git a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr new file mode 100644 index 0000000000000..85e8a600ce37f --- /dev/null +++ b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr @@ -0,0 +1,14 @@ +error: concrete type differs from previous defining opaque type use + --> $DIR/hidden_type_mismatch.rs:11:20 + | +LL | pub type Sep = impl Sized + std::fmt::Display; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, got `String` + | +note: previous use here + --> $DIR/hidden_type_mismatch.rs:37:21 + | +LL | pub type Tait = impl Copy + From> + Into>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/tests/ui/issues/issue-83190.rs b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs similarity index 100% rename from tests/ui/issues/issue-83190.rs rename to tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs index da931c3edaf6f..11b659eec9732 100644 --- a/tests/ui/issues/issue-83190.rs +++ b/tests/ui/type-alias-impl-trait/nested-rpit-with-lifetimes.rs @@ -1,7 +1,7 @@ -// check-pass - // Regression test for issue #83190, triggering an ICE in borrowck. +// check-pass + pub trait Any {} impl Any for T {}