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

WIP: eliminate DefineOpaqueTypes by using Yes across the compiler #127034

Closed
wants to merge 6 commits 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
13 changes: 12 additions & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_infer::infer::region_constraints::RegionConstraintData;
use rustc_infer::infer::{
BoundRegion, BoundRegionConversionTime, InferCtxt, NllRegionVariableOrigin,
};
use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
Expand All @@ -40,6 +41,7 @@ use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_span::DUMMY_SP;
use rustc_target::abi::{FieldIdx, FIRST_VARIANT};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::query::type_op::custom::scrape_region_constraints;
use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
use rustc_trait_selection::traits::query::type_op::{TypeOp, TypeOpOutput};
Expand Down Expand Up @@ -1799,7 +1801,16 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// `Sized` bound in no way depends on precise regions, so this
// shouldn't affect `is_sized`.
let erased_ty = tcx.erase_regions(ty);
if !erased_ty.is_sized(tcx, self.param_env) {
let sized_trait = tcx.require_lang_item(LangItem::Sized, Some(span));
let trait_ref = ty::TraitRef::new(tcx, sized_trait, [erased_ty]);

let is_sized = self.infcx.predicate_must_hold_modulo_regions(&Obligation::new(
tcx,
ObligationCause::dummy_with_span(span),
self.param_env,
trait_ref,
));
if !is_sized {
// in current MIR construction, all non-control-flow rvalue
// expressions evaluate through `as_temp` or `into` a return
// slot or local, so to find all unsized rvalues it is enough
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,42 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
predicate: self.infcx.resolve_vars_if_possible(obligation.predicate),
};

if obligation.predicate.skip_binder().self_ty().is_ty_var() {
debug!(ty = ?obligation.predicate.skip_binder().self_ty(), "ambiguous inference var or opaque type");
// Self is a type variable (e.g., `_: AsRef<str>`).
//
// This is somewhat problematic, as the current scheme can't really
// handle it turning to be a projection. This does end up as truly
// ambiguous in most cases anyway.
//
// Take the fast path out - this also improves
// performance by preventing assemble_candidates_from_impls from
// matching every impl for this trait.
return Ok(SelectionCandidateSet { vec: vec![], ambiguous: true });
}

let mut candidates = SelectionCandidateSet { vec: Vec::new(), ambiguous: false };
let def_id = obligation.predicate.def_id();
let tcx = self.tcx();

match obligation.predicate.skip_binder().self_ty().kind() {
// Opaque types in their defining scope are just like inference vars...
ty::Alias(ty::Opaque, alias) if self.infcx.can_define_opaque_ty(alias.def_id) => {
if tcx.is_lang_item(def_id, LangItem::Unsize) {
self.assemble_candidates_for_unsizing(obligation, &mut candidates);
}
self.assemble_candidates_from_impls(obligation, &mut candidates);
// .. unless we are looking for candidates just on the opaque signature, ...
self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
// .. or for auto traits, which look at the hidden type.
// Auto traits must be collected after projected tys, because opaque types
// do not emit auto trait candidates if a projection for the same auto trait
// already exists (e.g. due to the bounds on the opaque).
self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
return Ok(candidates);
}
ty::Infer(ty::TyVar(vid)) => {
debug!(?vid, "ambiguous inference var");
// Self is a type variable (e.g., `_: AsRef<str>`).
//
// This is somewhat problematic, as the current scheme can't really
// handle it turning to be a projection. This does end up as truly
// ambiguous in most cases anyway.
//
// Take the fast path out - this also improves
// performance by preventing assemble_candidates_from_impls from
// matching every impl for this trait.
candidates.ambiguous = true;
return Ok(candidates);
}
_ => {}
}

// Negative trait predicates have different rules than positive trait predicates.
if obligation.polarity() == ty::PredicatePolarity::Negative {
Expand All @@ -66,8 +87,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

// Other bounds. Consider both in-scope bounds from fn decl
// and applicable impls. There is a certain set of precedence rules here.
let def_id = obligation.predicate.def_id();
let tcx = self.tcx();

if tcx.is_lang_item(def_id, LangItem::Copy) {
debug!(obligation_self_ty = ?obligation.predicate.skip_binder().self_ty());
Expand Down Expand Up @@ -880,10 +899,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
return;
}
}
ty::Infer(ty::TyVar(_)) => {
debug!("assemble_candidates_from_object_ty: ambiguous");
candidates.ambiguous = true; // could wind up being an object type
return;
ty::Infer(ty::TyVar(_)) => bug!("assemble_candidates_from_object_ty is already gated behind an infer var check"),
ty::Alias(ty::Opaque, alias) if self.infcx.can_define_opaque_ty(alias.def_id) => {
bug!("assemble_candidates_from_object_ty is already gated behind an opaque type ");
}
_ => return,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligations.extend(
self.infcx
.at(&obligation.cause, obligation.param_env)
.eq(DefineOpaqueTypes::No, placeholder_trait_predicate, candidate)
.eq(DefineOpaqueTypes::Yes, placeholder_trait_predicate, candidate)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss these in #127035

.map(|InferOk { obligations, .. }| obligations)
.map_err(|_| Unimplemented)?,
);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
});
self.infcx
.at(&obligation.cause, obligation.param_env)
.eq(DefineOpaqueTypes::No, placeholder_trait_ref, trait_bound)
.eq(DefineOpaqueTypes::Yes, placeholder_trait_ref, trait_bound)
.map(|InferOk { obligations: _, value: () }| {
// This method is called within a probe, so we can't have
// inference variables and placeholders escape.
Expand Down Expand Up @@ -1752,7 +1752,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let is_match = self
.infcx
.at(&obligation.cause, obligation.param_env)
.eq(DefineOpaqueTypes::No, obligation.predicate, infer_projection)
.eq(DefineOpaqueTypes::Yes, obligation.predicate, infer_projection)
.is_ok_and(|InferOk { obligations, value: () }| {
self.evaluate_predicates_recursively(
TraitObligationStackList::empty(&ProvisionalEvaluationCache::default()),
Expand Down Expand Up @@ -2532,7 +2532,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
let InferOk { obligations, .. } = self
.infcx
.at(&cause, obligation.param_env)
.eq(DefineOpaqueTypes::No, placeholder_obligation_trait_ref, impl_trait_ref)
.eq(DefineOpaqueTypes::Yes, placeholder_obligation_trait_ref, impl_trait_ref)
.map_err(|e| {
debug!("match_impl: failed eq_trait_refs due to `{}`", e.to_string(self.tcx()))
})?;
Expand Down Expand Up @@ -2675,7 +2675,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
);
self.infcx
.at(&obligation.cause, obligation.param_env)
.eq(DefineOpaqueTypes::No, predicate.trait_ref, trait_ref)
.eq(DefineOpaqueTypes::Yes, predicate.trait_ref, trait_ref)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used in selection (behind a probe) and confirmation. Without the previous commit skipping assemble_candidates_from_caller_bounds for opaque types in their defining scope, this will cause functions like

fn filter_fold<T, Acc>(
mut predicate: impl FnMut(&T) -> bool,
mut fold: impl FnMut(Acc, T) -> Acc,
) -> impl FnMut(Acc, T) -> Acc {
move |acc, item| if predicate(&item) { fold(acc, item) } else { acc }
}

to immediately match the opaque type against the APIT with the same bounds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've looked into it a bit. Do you know of any fundamental issues with moving the old solver to a similar normalization scheme for opaques as the new solver uses? Or is that just a bad version of lazy norm?

.map(|InferOk { obligations, .. }| obligations)
.map_err(|_| ())
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0284]: type annotations needed: cannot satisfy `Foo == _`
--> $DIR/norm-before-method-resolution-opaque-type.rs:16:19
--> $DIR/norm-before-method-resolution-opaque-type.rs:15:19
|
LL | fn weird_bound<X>(x: &<X as Trait<'static>>::Out<Foo>) -> X
| ^ cannot satisfy `Foo == _`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,11 @@
error: item does not constrain `Foo::{opaque#0}`, but has it in its signature
--> $DIR/norm-before-method-resolution-opaque-type.rs:16:4
error[E0161]: cannot move a value of type `<X as Trait<'_>>::Out<Foo>`
--> $DIR/norm-before-method-resolution-opaque-type.rs:21:9
|
LL | fn weird_bound<X>(x: &<X as Trait<'static>>::Out<Foo>) -> X
| ^^^^^^^^^^^
|
= note: consider moving the opaque type's declaration and defining uses into a separate module
note: this opaque type is in the signature
--> $DIR/norm-before-method-resolution-opaque-type.rs:13:12
|
LL | type Foo = impl Sized;
| ^^^^^^^^^^

error: unconstrained opaque type
--> $DIR/norm-before-method-resolution-opaque-type.rs:13:12
|
LL | type Foo = impl Sized;
| ^^^^^^^^^^
|
= note: `Foo` must be used in combination with a concrete type within the same module
LL | let x = *x;
| ^ the size of `<X as Trait<'_>>::Out<Foo>` cannot be statically determined

error[E0507]: cannot move out of `*x` which is behind a shared reference
--> $DIR/norm-before-method-resolution-opaque-type.rs:23:13
--> $DIR/norm-before-method-resolution-opaque-type.rs:21:13
|
LL | let x = *x;
| ^^ move occurs because `*x` has type `<X as Trait<'_>>::Out<Foo>`, which does not implement the `Copy` trait
Expand All @@ -31,6 +16,7 @@ LL - let x = *x;
LL + let x = x;
|

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

For more information about this error, try `rustc --explain E0507`.
Some errors have detailed explanations: E0161, E0507.
For more information about an error, try `rustc --explain E0161`.
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@ impl<'a, T> Trait<'a> for T {
}

type Foo = impl Sized;
//[old]~^ ERROR: unconstrained opaque type

fn weird_bound<X>(x: &<X as Trait<'static>>::Out<Foo>) -> X
//[old]~^ ERROR: item does not constrain
//[next]~^^ ERROR: cannot satisfy `Foo == _`
//[next]~^ ERROR: cannot satisfy `Foo == _`
where
for<'a> X: Trait<'a>,
for<'a> <X as Trait<'a>>::Out<()>: Copy,
{
let x = *x; //[old]~ ERROR: cannot move out of `*x`
//[old]~^ ERROR: cannot move a value of type `<X as Trait<'_>>::Out<Foo>`
todo!();
}

Expand Down
24 changes: 24 additions & 0 deletions tests/ui/impl-trait/bind-hidden-type-in-projection-matching.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#![feature(type_alias_impl_trait)]
trait Trait<'a> {
type Out<U>;
}

impl<'a, T> Trait<'a> for T {
type Out<U> = T;
}

type Foo = impl Sized;

fn weird_bound<X>(x: &<X as Trait<'static>>::Out<Foo>) -> X
where
for<'a> X: Trait<'a>,
for<'a> <X as Trait<'a>>::Out<()>: Copy,
{
let x = *x; //~ ERROR: cannot move out of `*x`
//~^ ERROR: cannot move a value of type `<X as Trait<'_>>::Out<Foo>`
todo!();
}

fn main() {
let _: () = weird_bound(&());
}
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/bind-hidden-type-in-projection-matching.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0161]: cannot move a value of type `<X as Trait<'_>>::Out<Foo>`
--> $DIR/bind-hidden-type-in-projection-matching.rs:17:9
|
LL | let x = *x;
| ^ the size of `<X as Trait<'_>>::Out<Foo>` cannot be statically determined

error[E0507]: cannot move out of `*x` which is behind a shared reference
--> $DIR/bind-hidden-type-in-projection-matching.rs:17:13
|
LL | let x = *x;
| ^^ move occurs because `*x` has type `<X as Trait<'_>>::Out<Foo>`, which does not implement the `Copy` trait
|
help: consider removing the dereference here
|
LL - let x = *x;
LL + let x = x;
|

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0161, E0507.
For more information about an error, try `rustc --explain E0161`.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0282]: type annotations needed for `&_`
--> $DIR/call_method_on_inherent_impl_on_rigid_type.rs:14:13
--> $DIR/call_method_on_inherent_impl_on_rigid_type.rs:15:13
|
LL | let x = &my_foo();
| ^
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ revisions: current next
//@[next] compile-flags: -Znext-solver
//@[current] check-pass

trait MyDebug {
fn my_debug(&self);
Expand All @@ -14,7 +15,6 @@ fn my_foo() -> impl std::fmt::Debug {
let x = &my_foo();
//[next]~^ ERROR: type annotations needed
x.my_debug();
//[current]~^ ERROR: no method named `my_debug`
}
()
}
Expand Down
13 changes: 8 additions & 5 deletions tests/ui/impl-trait/equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn sum_to(n: u32) -> impl Foo {
0
} else {
n + sum_to(n - 1)
//~^ ERROR cannot add `impl Foo` to `u32`
//~^ ERROR cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`
}
}

Expand All @@ -32,12 +32,15 @@ trait Leak: Sized {
}
impl<T> Leak for T {
default type T = ();
default fn leak(self) -> Self::T { panic!() }
default fn leak(self) -> Self::T {
panic!()
}
}
impl Leak for i32 {
type T = i32;
fn leak(self) -> i32 { self }
fn leak(self) -> i32 {
self
}
}

fn main() {
}
fn main() {}
15 changes: 4 additions & 11 deletions tests/ui/impl-trait/equality.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,13 @@ help: change the type of the numeric literal from `u32` to `i32`
LL | 0_i32
| ~~~

error[E0277]: cannot add `impl Foo` to `u32`
error[E0284]: type annotations needed: cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`
--> $DIR/equality.rs:24:11
|
LL | n + sum_to(n - 1)
| ^ no implementation for `u32 + impl Foo`
|
= help: the trait `Add<impl Foo>` is not implemented for `u32`
= help: the following other types implement trait `Add<Rhs>`:
`&'a u32` implements `Add<u32>`
`&u32` implements `Add<&u32>`
`u32` implements `Add<&u32>`
`u32` implements `Add`
| ^ cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`

error: aborting due to 2 previous errors; 1 warning emitted

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
Some errors have detailed explanations: E0284, E0308.
For more information about an error, try `rustc --explain E0284`.
2 changes: 0 additions & 2 deletions tests/ui/impl-trait/nested_impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ fn fine(x: impl Into<u32>) -> impl Into<u32> { x }

fn bad_in_ret_position(x: impl Into<u32>) -> impl Into<impl Debug> { x }
//~^ ERROR nested `impl Trait` is not allowed
//~| ERROR the trait bound `impl Debug: From<impl Into<u32>>` is not satisfied

fn bad_in_fn_syntax(x: fn() -> impl Into<impl Debug>) {}
//~^ ERROR nested `impl Trait` is not allowed
Expand All @@ -18,7 +17,6 @@ struct X;
impl X {
fn bad(x: impl Into<u32>) -> impl Into<impl Debug> { x }
//~^ ERROR nested `impl Trait` is not allowed
//~| ERROR the trait bound `impl Debug: From<impl Into<u32>>` is not satisfied
}

fn allowed_in_assoc_type() -> impl Iterator<Item=impl Fn()> {
Expand Down
Loading
Loading