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

implied bounds: normalize in the proper param_env #105982

Closed
wants to merge 1 commit into from
Closed
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
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
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/subst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
32 changes: 4 additions & 28 deletions compiler/rustc_trait_selection/src/traits/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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>(
Expand Down
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
Expand Down Expand Up @@ -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`
|
Expand Down
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`
|
Expand Down
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`
|
Expand Down
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`
|
Expand Down
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`
|
Expand Down
14 changes: 5 additions & 9 deletions src/test/ui/associated-types/issue-59324.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
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
Expand Down
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
Expand Up @@ -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!()
}

Expand Down
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
Expand Up @@ -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