Skip to content

implied bounds: normalize in the proper param_env #105982

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

Closed
wants to merge 1 commit into from
Closed
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
3 changes: 1 addition & 2 deletions compiler/rustc_hir_analysis/src/check/compare_method.rs
Original file line number Diff line number Diff line change
@@ -1803,8 +1803,7 @@ pub fn check_type_bounds<'tcx>(
let infcx = tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(&infcx);

let assumed_wf_types =
ocx.assumed_wf_types(param_env, impl_ty_span, impl_ty.def_id.expect_local());
let assumed_wf_types = ocx.assumed_wf_types(impl_ty.def_id.expect_local());

let normalize_cause = ObligationCause::new(
impl_ty_span,
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -97,7 +97,7 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
let infcx = &tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(infcx);

let assumed_wf_types = ocx.assumed_wf_types(param_env, span, body_def_id);
let assumed_wf_types = ocx.assumed_wf_types(body_def_id);

let mut wfcx = WfCheckingCtxt { ocx, span, body_id, param_env };

Original file line number Diff line number Diff line change
@@ -167,8 +167,7 @@ fn get_impl_substs<'tcx>(
let param_env = tcx.param_env(impl1_def_id);
let impl1_hir_id = tcx.hir().local_def_id_to_hir_id(impl1_def_id);

let assumed_wf_types =
ocx.assumed_wf_types(param_env, tcx.def_span(impl1_def_id), impl1_def_id);
let assumed_wf_types = ocx.assumed_wf_types(impl1_def_id);

let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id.to_def_id());
let impl2_substs =
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -800,10 +800,9 @@ rustc_queries! {
}

/// Returns the types assumed to be well formed while "inside" of the given item.
///
/// Note that we've liberated the late bound regions of function signatures, so
/// this can not be used to check whether these types are well formed.
query assumed_wf_types(key: DefId) -> &'tcx ty::List<Ty<'tcx>> {
query assumed_wf_types(
key: DefId
) -> ty::EarlyBinder<ty::Binder<'tcx, &'tcx ty::List<Ty<'tcx>>>> {
desc { |tcx| "computing the implied bounds of `{}`", tcx.def_path_str(key) }
}

4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/subst.rs
Original file line number Diff line number Diff line change
@@ -705,6 +705,10 @@ impl<'tcx, T: TypeFoldable<'tcx>> ty::EarlyBinder<T> {
let mut folder = SubstFolder { tcx, substs, binders_passed: 0 };
self.0.fold_with(&mut folder)
}

pub fn subst_identity(self) -> T {
self.0
}
}

///////////////////////////////////////////////////////////////////////////
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
@@ -389,7 +389,7 @@ fn resolve_negative_obligation<'tcx>(
};

let ocx = ObligationCtxt::new(&infcx);
let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id);
let wf_tys = ocx.assumed_wf_types(body_def_id);
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
Some(&infcx),
32 changes: 4 additions & 28 deletions compiler/rustc_trait_selection/src/traits/engine.rs
Original file line number Diff line number Diff line change
@@ -20,7 +20,6 @@ use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::ToPredicate;
use rustc_middle::ty::TypeFoldable;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::Span;

pub trait TraitEngineExt<'tcx> {
fn new(tcx: TyCtxt<'tcx>) -> Box<Self>;
@@ -179,34 +178,11 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> {
self.engine.borrow_mut().select_all_or_error(self.infcx)
}

pub fn assumed_wf_types(
&self,
param_env: ty::ParamEnv<'tcx>,
span: Span,
def_id: LocalDefId,
) -> FxIndexSet<Ty<'tcx>> {
pub fn assumed_wf_types(&self, def_id: LocalDefId) -> FxIndexSet<Ty<'tcx>> {
let tcx = self.infcx.tcx;
let assumed_wf_types = tcx.assumed_wf_types(def_id);
let mut implied_bounds = FxIndexSet::default();
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
let cause = ObligationCause::misc(span, hir_id);
for ty in assumed_wf_types {
// FIXME(@lcnr): rustc currently does not check wf for types
// pre-normalization, meaning that implied bounds are sometimes
// incorrect. See #100910 for more details.
//
// Not adding the unnormalized types here mostly fixes that, except
// that there are projections which are still ambiguous in the item definition
// but do normalize successfully when using the item, see #98543.
//
// Anyways, I will hopefully soon change implied bounds to make all of this
// sound and then uncomment this line again.

// implied_bounds.insert(ty);
let normalized = self.normalize(&cause, param_env, ty);
implied_bounds.insert(normalized);
}
implied_bounds
let def_id = def_id.to_def_id();
let assumed_wf_types = tcx.assumed_wf_types(def_id).subst_identity();
tcx.liberate_late_bound_regions(def_id, assumed_wf_types).into_iter().collect()
}

pub fn make_canonicalized_query_response<T>(
87 changes: 65 additions & 22 deletions compiler/rustc_ty_utils/src/implied_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,45 @@
use crate::rustc_middle::ty::DefIdTree;
use rustc_hir::{def::DefKind, def_id::DefId};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::traits::query::NoSolution;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};

pub fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers { assumed_wf_types, ..*providers };
}

fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx ty::List<Ty<'tcx>> {
fn assumed_wf_types<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> ty::EarlyBinder<ty::Binder<'tcx, &'tcx ty::List<Ty<'tcx>>>> {
match tcx.def_kind(def_id) {
DefKind::Fn => {
let sig = tcx.fn_sig(def_id);
let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
liberated_sig.inputs_and_output
}
DefKind::Fn => ty::EarlyBinder(from_fn_sig(tcx, def_id)),
DefKind::AssocFn => {
let sig = tcx.fn_sig(def_id);
let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
let mut assumed_wf_types: Vec<_> =
tcx.assumed_wf_types(tcx.parent(def_id)).as_slice().into();
assumed_wf_types.extend(liberated_sig.inputs_and_output);
tcx.intern_type_list(&assumed_wf_types)
let from_sig = from_fn_sig(tcx, def_id);
let from_impl = tcx.assumed_wf_types(tcx.parent(def_id)).subst_identity();

let assumed_wf_types = from_impl.no_bound_vars().unwrap().into_iter();
let assumed_wf_types = assumed_wf_types.chain(from_sig.skip_binder());
ty::EarlyBinder(from_sig.rebind(tcx.mk_type_list(assumed_wf_types)))
}
DefKind::Impl => {
let unnormalized = match tcx.impl_trait_ref(def_id) {
Some(trait_ref) => tcx.mk_type_list(trait_ref.substs.types()),
// Only the impl self type
None => tcx.intern_type_list(&[tcx.type_of(def_id)]),
};

// FIXME(@lcnr): rustc currently does not check wf for types
// pre-normalization, meaning that implied bounds from unnormalized types
// are sometimes incorrect. See #100910 for more details.
//
// Not adding the unnormalized types here mostly fixes that, except
// that there are projections which are still ambiguous in the item definition
// but do normalize successfully when using the item, see #98543.
let normalized =
normalize_ignoring_obligations(tcx, tcx.param_env(def_id), unnormalized)
.unwrap_or_else(|_| ty::List::empty());
ty::EarlyBinder(ty::Binder::dummy(normalized))
}
DefKind::Impl => match tcx.impl_trait_ref(def_id) {
Some(trait_ref) => {
let types: Vec<_> = trait_ref.substs.types().collect();
tcx.intern_type_list(&types)
}
// Only the impl self type
None => tcx.intern_type_list(&[tcx.type_of(def_id)]),
},
DefKind::AssocConst | DefKind::AssocTy => tcx.assumed_wf_types(tcx.parent(def_id)),
DefKind::Mod
| DefKind::Struct
@@ -56,6 +67,38 @@ fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx ty::List<Ty
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure
| DefKind::Generator => ty::List::empty(),
| DefKind::Generator => ty::EarlyBinder(ty::Binder::dummy(ty::List::empty())),
}
}

fn from_fn_sig<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> ty::Binder<'tcx, &'tcx ty::List<Ty<'tcx>>> {
// FIXME(#84533): We probably shouldn't use output types in implied bounds.
// This would reject this fn `fn f<'a, 'b>() -> &'a &'b () { .. }`.
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should be how we fix #84533 because you'd still have an builtin impl with an associated type which isn't well formed.

The correct fix is to actually check that the return type is well formed even without calling the function. This needs implications for binders though.

Until then something like #106807 might work as a stopgap (again having the issue of unnormalized projections though)

let unnormalized = tcx.fn_sig(def_id).map_bound(|sig| sig.inputs_and_output);
let normalized = normalize_ignoring_obligations(tcx, tcx.param_env(def_id), unnormalized)
.unwrap_or_else(|_| ty::Binder::dummy(ty::List::empty()));

// FIXME(#105948): Use unnormalized types for implied bounds as well.
normalized
}

fn normalize_ignoring_obligations<'tcx, T: TypeFoldable<'tcx>>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: T,
) -> Result<T, NoSolution> {
use rustc_infer::infer::TyCtxtInferExt as _;
use rustc_infer::traits::ObligationCause;
use rustc_trait_selection::traits::query::normalize::QueryNormalizeExt as _;

let infcx = tcx.infer_ctxt().build();
let normalized = infcx
.at(&ObligationCause::dummy(), param_env)
.query_normalize(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

query_normalize eagerly evaluates constants causing cycle errors if there are unevaluated constants in the fn sig.

During analysis normalize should be used instead.

.map(|normalized| infcx.resolve_vars_if_possible(normalized.value))?;
assert!(!normalized.needs_infer());
Ok(normalized)
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `Self: Get` is not satisfied
--> $DIR/associated-types-for-unimpl-trait.rs:10:5
--> $DIR/associated-types-for-unimpl-trait.rs:10:40
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
help: consider further restricting `Self`
|
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `T: Get` is not satisfied
--> $DIR/associated-types-no-suitable-bound.rs:11:5
--> $DIR/associated-types-no-suitable-bound.rs:11:21
|
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
| ^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
|
help: consider restricting type parameter `T`
|
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `Self: Get` is not satisfied
--> $DIR/associated-types-no-suitable-supertrait-2.rs:17:5
--> $DIR/associated-types-no-suitable-supertrait-2.rs:17:40
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
help: consider further restricting `Self`
|
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
error[E0277]: the trait bound `(T, U): Get` is not satisfied
--> $DIR/associated-types-no-suitable-supertrait.rs:22:5
--> $DIR/associated-types-no-suitable-supertrait.rs:22:40
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <(T, U) as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `(T, U)`
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `(T, U)`

error[E0277]: the trait bound `Self: Get` is not satisfied
--> $DIR/associated-types-no-suitable-supertrait.rs:17:5
--> $DIR/associated-types-no-suitable-supertrait.rs:17:40
|
LL | fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
help: consider further restricting `Self`
|
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `Self: Get` is not satisfied
--> $DIR/associated-types-projection-to-unrelated-trait-in-method-without-default.rs:10:5
--> $DIR/associated-types-projection-to-unrelated-trait-in-method-without-default.rs:10:40
|
LL | fn okay<U:Get>(&self, foo: U, bar: <Self as Get>::Value);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
| ^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `Self`
|
help: consider further restricting `Self`
|
14 changes: 5 additions & 9 deletions src/test/ui/associated-types/issue-59324.stderr
Original file line number Diff line number Diff line change
@@ -45,20 +45,16 @@ LL | pub trait ThriftService<Bug: NotFoo + Foo>:
| +++++

error[E0277]: the trait bound `(): Foo` is not satisfied
--> $DIR/issue-59324.rs:23:1
--> $DIR/issue-59324.rs:23:29
|
LL | fn with_factory<H>(factory: dyn ThriftService<()>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo` is not implemented for `()`
| ^^^^^^^^^^^^^^^^^^^^^ the trait `Foo` is not implemented for `()`

error[E0277]: the trait bound `Bug: Foo` is not satisfied
--> $DIR/issue-59324.rs:16:5
--> $DIR/issue-59324.rs:16:8
|
LL | / fn get_service(
LL | |
LL | |
LL | | &self,
LL | | ) -> Self::AssocType;
| |_________________________^ the trait `Foo` is not implemented for `Bug`
LL | fn get_service(
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `Bug`
|
help: consider further restricting this bound
|
Original file line number Diff line number Diff line change
@@ -15,11 +15,11 @@ note: ...which requires type-checking `test::{constant#0}`...
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
| ^^^^^^^^^^^^^
= note: ...which again requires building an abstract representation for `test::{constant#0}`, completing the cycle
note: cycle used when checking that `test` is well-formed
--> $DIR/closures.rs:3:1
note: cycle used when building MIR for `test::{constant#0}`
--> $DIR/closures.rs:3:35
|
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
error: overly complex generic constant
--> $DIR/let-bindings.rs:6:68
--> $DIR/let-bindings.rs:6:35
|
LL | fn test<const N: usize>() -> [u8; { let x = N; N + 1 }] where [u8; { let x = N; N + 1 }]: Default {
| ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant
| ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant
|
= help: consider moving this anonymous constant into a `const` function
= note: this operation may be supported in the future

error: overly complex generic constant
--> $DIR/let-bindings.rs:6:35
--> $DIR/let-bindings.rs:6:68
|
LL | fn test<const N: usize>() -> [u8; { let x = N; N + 1 }] where [u8; { let x = N; N + 1 }]: Default {
| ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant
| ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant
|
= help: consider moving this anonymous constant into a `const` function
= note: this operation may be supported in the future
1 change: 1 addition & 0 deletions src/test/ui/const-generics/issues/issue-77357.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ trait MyTrait<T> {}

fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
//~^ ERROR overly complex generic constant
//~| ERROR cycle detected when evaluating type-level constant
todo!()
}

42 changes: 41 additions & 1 deletion src/test/ui/const-generics/issues/issue-77357.stderr
Original file line number Diff line number Diff line change
@@ -7,5 +7,45 @@ LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
= help: consider moving this anonymous constant into a `const` function
= note: this operation may be supported in the future

error: aborting due to previous error
error[E0391]: cycle detected when evaluating type-level constant
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires const-evaluating + checking `bug::{constant#0}`...
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires const-evaluating + checking `bug::{constant#0}`...
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires caching mir of `bug::{constant#0}` for CTFE...
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires elaborating drops for `bug::{constant#0}`...
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires borrow-checking `bug::{constant#0}`...
--> $DIR/issue-77357.rs:6:46
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires normalizing `Binder(ConstEvaluatable(Const { ty: usize, kind: Unevaluated(UnevaluatedConst { def: WithOptConstParam { did: DefId(0:8 ~ issue_77357[fee6]::bug::{constant#0}), const_param_did: None }, substs: [T] }) }), [])`...
= note: ...which again requires evaluating type-level constant, completing the cycle
note: cycle used when computing the implied bounds of `bug`
--> $DIR/issue-77357.rs:6:1
|
LL | fn bug<'a, T>() -> &'static dyn MyTrait<[(); { |x: &'a u32| { x }; 4 }]> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0391`.
Loading