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

Check hidden types for well formedness at the definition site instead of only at the opaque type itself #96736

Merged
merged 1 commit into from
May 10, 2022
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
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let remapped_type = infcx.infer_opaque_definition_from_instantiation(
opaque_type_key,
universal_concrete_type,
origin,
);
let ty = if check_opaque_type_parameter_valid(
infcx.tcx,
Expand Down
74 changes: 71 additions & 3 deletions compiler/rustc_trait_selection/src/opaque_types.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
use crate::traits;
use crate::traits::error_reporting::InferCtxtExt as _;
use crate::traits::TraitEngineExt as _;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::DefId;
use rustc_hir::OpaqueTyOrigin;
use rustc_infer::infer::error_reporting::unexpected_hidden_region_diagnostic;
use rustc_infer::infer::InferCtxt;
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt as _};
use rustc_infer::traits::{Obligation, ObligationCause, TraitEngine};
use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts};
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt};
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, ToPredicate, Ty, TyCtxt};
use rustc_span::Span;

pub trait InferCtxtExt<'tcx> {
fn infer_opaque_definition_from_instantiation(
&self,
opaque_type_key: OpaqueTypeKey<'tcx>,
instantiated_ty: OpaqueHiddenType<'tcx>,
origin: OpaqueTyOrigin,
) -> Ty<'tcx>;
}

Expand Down Expand Up @@ -45,6 +50,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&self,
opaque_type_key: OpaqueTypeKey<'tcx>,
instantiated_ty: OpaqueHiddenType<'tcx>,
origin: OpaqueTyOrigin,
) -> Ty<'tcx> {
if self.is_tainted_by_errors() {
return self.tcx.ty_error();
Expand Down Expand Up @@ -76,7 +82,69 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
));
debug!(?definition_ty);

definition_ty
// Only check this for TAIT. RPIT already supports `src/test/ui/impl-trait/nested-return-type2.rs`
// on stable and we'd break that.
if let OpaqueTyOrigin::TyAlias = origin {
// This logic duplicates most of `check_opaque_meets_bounds`.
// FIXME(oli-obk): Also do region checks here and then consider removing `check_opaque_meets_bounds` entirely.
let param_env = self.tcx.param_env(def_id);
let body_id = self.tcx.local_def_id_to_hir_id(def_id.as_local().unwrap());
self.tcx.infer_ctxt().enter(move |infcx| {
// Require the hidden type to be well-formed with only the generics of the opaque type.
// Defining use functions may have more bounds than the opaque type, which is ok, as long as the
// hidden type is well formed even without those bounds.
let predicate =
ty::Binder::dummy(ty::PredicateKind::WellFormed(definition_ty.into()))
.to_predicate(infcx.tcx);
let mut fulfillment_cx = <dyn TraitEngine<'tcx>>::new(infcx.tcx);

// Require that the hidden type actually fulfills all the bounds of the opaque type, even without
// the bounds that the function supplies.
match infcx.register_hidden_type(
OpaqueTypeKey { def_id, substs: id_substs },
ObligationCause::misc(instantiated_ty.span, body_id),
param_env,
definition_ty,
origin,
) {
Ok(infer_ok) => {
for obligation in infer_ok.obligations {
fulfillment_cx.register_predicate_obligation(&infcx, obligation);
}
}
Err(err) => {
infcx
.report_mismatched_types(
&ObligationCause::misc(instantiated_ty.span, body_id),
self.tcx.mk_opaque(def_id, id_substs),
definition_ty,
err,
)
.emit();
}
}

fulfillment_cx.register_predicate_obligation(
&infcx,
Obligation::misc(instantiated_ty.span, body_id, param_env, predicate),
);

// Check that all obligations are satisfied by the implementation's
// version.
let errors = fulfillment_cx.select_all_or_error(&infcx);

let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();

if errors.is_empty() {
definition_ty
} else {
infcx.report_fulfillment_errors(&errors, None, false);
self.tcx.ty_error()
}
})
} else {
definition_ty
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/type-alias-impl-trait/bounds-are-checked-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
#![feature(type_alias_impl_trait)]

type X<T> = impl Clone;
//~^ ERROR the trait bound `T: Clone` is not satisfied

fn f<T: Clone>(t: T) -> X<T> {
t
//~^ ERROR the trait bound `T: Clone` is not satisfied
}

fn g<T>(o: Option<X<T>>) -> Option<X<T>> {
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/type-alias-impl-trait/bounds-are-checked-2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `T: Clone` is not satisfied
--> $DIR/bounds-are-checked-2.rs:6:13
--> $DIR/bounds-are-checked-2.rs:9:5
|
LL | type X<T> = impl Clone;
| ^^^^^^^^^^ the trait `Clone` is not implemented for `T`
LL | t
| ^ the trait `Clone` is not implemented for `T`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's slightly confusing that we're talking about T here and f also has a T, but considering the help right below, that should be explanatory enough for this PR I think.

I want to add a new type-alias-impl-trait ObligationCause that we will then use to improve this diagnostic.

|
help: consider restricting type parameter `T`
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type TwoConsts<const X: usize, const Y: usize> = impl Debug;
fn one_ty<T: Debug>(t: T) -> TwoTys<T, T> {
t
//~^ ERROR non-defining opaque type use in defining scope
//~| ERROR `U` doesn't implement `Debug`
}

fn one_lifetime<'a>(t: &'a u32) -> TwoLifetimes<'a, 'a> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
error[E0277]: `U` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use.rs:16:5
|
LL | t
| ^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
help: consider restricting type parameter `U`
|
LL | type TwoTys<T, U: std::fmt::Debug> = impl Debug;
| +++++++++++++++++

error: non-defining opaque type use in defining scope
--> $DIR/generic_duplicate_param_use.rs:16:5
|
Expand All @@ -11,7 +22,7 @@ LL | type TwoTys<T, U> = impl Debug;
| ^ ^

error: non-defining opaque type use in defining scope
--> $DIR/generic_duplicate_param_use.rs:21:5
--> $DIR/generic_duplicate_param_use.rs:22:5
|
LL | t
| ^
Expand All @@ -23,7 +34,7 @@ LL | type TwoLifetimes<'a, 'b> = impl Debug;
| ^^ ^^

error: non-defining opaque type use in defining scope
--> $DIR/generic_duplicate_param_use.rs:26:5
--> $DIR/generic_duplicate_param_use.rs:27:5
|
LL | t
| ^
Expand All @@ -34,5 +45,6 @@ note: constant used multiple times
LL | type TwoConsts<const X: usize, const Y: usize> = impl Debug;
| ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ fn main() {}

// test that unused generic parameters are ok
type Two<T, U> = impl Debug;
//~^ ERROR `T` doesn't implement `Debug`

fn two<T: Debug, U>(t: T, _: U) -> Two<T, U> {
t
//~^ ERROR `T` doesn't implement `Debug`
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: `T` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use2.rs:8:18
--> $DIR/generic_duplicate_param_use2.rs:11:5
|
LL | type Two<T, U> = impl Debug;
| ^^^^^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
LL | t
| ^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
help: consider restricting type parameter `T`
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ fn main() {}

// test that unused generic parameters are ok
type Two<T, U> = impl Debug;
//~^ ERROR `T` doesn't implement `Debug`

fn two<T: Debug, U>(t: T, _: U) -> Two<T, U> {
t
//~^ ERROR `T` doesn't implement `Debug`
}

fn three<T, U: Debug>(_: T, u: U) -> Two<T, U> {
u
//~^ ERROR concrete type differs from previous defining opaque type use
//~^ ERROR `U` doesn't implement `Debug`
}
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
error: concrete type differs from previous defining opaque type use
--> $DIR/generic_duplicate_param_use3.rs:16:5
|
LL | u
| ^ expected `T`, got `U`
|
note: previous use here
--> $DIR/generic_duplicate_param_use3.rs:12:5
|
LL | t
| ^

error[E0277]: `T` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use3.rs:8:18
--> $DIR/generic_duplicate_param_use3.rs:11:5
|
LL | type Two<T, U> = impl Debug;
| ^^^^^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
LL | t
| ^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
help: consider restricting type parameter `T`
|
LL | type Two<T: std::fmt::Debug, U> = impl Debug;
| +++++++++++++++++

error[E0277]: `U` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use3.rs:16:5
|
LL | u
| ^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
help: consider restricting type parameter `U`
|
LL | type Two<T, U: std::fmt::Debug> = impl Debug;
| +++++++++++++++++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ fn main() {}

// test that unused generic parameters are ok
type Two<T, U> = impl Debug;
//~^ ERROR `U` doesn't implement `Debug`

fn three<T, U: Debug>(_: T, u: U) -> Two<T, U> {
u
//~^ ERROR `U` doesn't implement `Debug`
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: `U` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use4.rs:8:18
--> $DIR/generic_duplicate_param_use4.rs:11:5
|
LL | type Two<T, U> = impl Debug;
| ^^^^^^^^^^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
LL | u
| ^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
help: consider restricting type parameter `U`
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ fn main() {}

// test that unused generic parameters are ok
type Two<T, U> = impl Debug;
//~^ ERROR `T` doesn't implement `Debug`
//~| ERROR `U` doesn't implement `Debug`

fn two<T: Debug, U: Debug>(t: T, u: U) -> Two<T, U> {
(t, u)
//~^ ERROR `T` doesn't implement `Debug`
//~| ERROR `U` doesn't implement `Debug`
}

fn three<T: Debug, U: Debug>(t: T, u: U) -> Two<T, U> {
(u, t)
//~^ concrete type differs from previous
//~^ ERROR `T` doesn't implement `Debug`
//~| ERROR `U` doesn't implement `Debug`
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,8 @@
error: concrete type differs from previous defining opaque type use
--> $DIR/generic_duplicate_param_use5.rs:17:5
|
LL | (u, t)
| ^^^^^^ expected `(T, U)`, got `(U, T)`
|
note: previous use here
--> $DIR/generic_duplicate_param_use5.rs:13:5
|
LL | (t, u)
| ^^^^^^

error[E0277]: `T` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use5.rs:8:18
--> $DIR/generic_duplicate_param_use5.rs:11:5
|
LL | type Two<T, U> = impl Debug;
| ^^^^^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
LL | (t, u)
| ^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= note: required because of the requirements on the impl of `Debug` for `(T, U)`
help: consider restricting type parameter `T`
Expand All @@ -23,17 +11,41 @@ LL | type Two<T: std::fmt::Debug, U> = impl Debug;
| +++++++++++++++++

error[E0277]: `U` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use5.rs:8:18
--> $DIR/generic_duplicate_param_use5.rs:11:5
|
LL | type Two<T, U> = impl Debug;
| ^^^^^^^^^^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
LL | (t, u)
| ^^^^^^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= note: required because of the requirements on the impl of `Debug` for `(T, U)`
help: consider restricting type parameter `U`
|
LL | type Two<T, U: std::fmt::Debug> = impl Debug;
| +++++++++++++++++

error: aborting due to 3 previous errors
error[E0277]: `U` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use5.rs:17:5
|
LL | (u, t)
| ^^^^^^ `U` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= note: required because of the requirements on the impl of `Debug` for `(U, T)`
help: consider restricting type parameter `U`
|
LL | type Two<T, U: std::fmt::Debug> = impl Debug;
| +++++++++++++++++

error[E0277]: `T` doesn't implement `Debug`
--> $DIR/generic_duplicate_param_use5.rs:17:5
|
LL | (u, t)
| ^^^^^^ `T` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= note: required because of the requirements on the impl of `Debug` for `(U, T)`
help: consider restricting type parameter `T`
|
LL | type Two<T: std::fmt::Debug, U> = impl Debug;
| +++++++++++++++++

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ fn main() {}

// test that unused generic parameters are ok
type Two<T, U> = impl Debug;
//~^ ERROR `T` doesn't implement `Debug`

fn two<T: Copy + Debug, U: Debug>(t: T, u: U) -> Two<T, U> {
(t, t)
//~^ ERROR `T` doesn't implement `Debug`
}

fn three<T: Copy + Debug, U: Debug>(t: T, u: U) -> Two<T, U> {
(u, t)
//~^ ERROR concrete type differs from previous
//~^ ERROR `T` doesn't implement `Debug`
//~| ERROR `U` doesn't implement `Debug`
}
Loading