Skip to content

Fix default methods with return-position impl Trait in trait #107013

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
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
20 changes: 19 additions & 1 deletion compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
//! `RETURN_PLACE` the MIR arguments) are always fully normalized (and
//! contain revealed `impl Trait` values).

use rustc_hir::def::DefKind;
use rustc_index::vec::Idx;
use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{self, Ty, TypeFoldable};
use rustc_span::Span;

use crate::universal_regions::UniversalRegions;
Expand Down Expand Up @@ -148,6 +150,22 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

#[instrument(skip(self), level = "debug")]
fn equate_normalized_input_or_output(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, span: Span) {
let (a, b) = (a, b).fold_with(&mut BottomUpFolder {
Copy link
Member Author

Choose a reason for hiding this comment

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

Within a method, treat its own RPITITs as ty::Opaque.

This probably could be a custom folder so it's more efficient... perf will tell.

tcx: self.infcx.tcx,
ty_op: |ty| {
if let ty::Alias(ty::Projection, ty::AliasTy { def_id, substs, .. }) = *ty.kind()
&& self.infcx.tcx.def_kind(def_id) == DefKind::ImplTraitPlaceholder
&& def_id.as_local().map_or(false, |def_id| self.infcx.opaque_type_origin(def_id, span).is_some())
{
self.infcx.tcx.mk_opaque(def_id, substs)
} else {
ty
}
},
lt_op: |lt| lt,
ct_op: |ct| ct,
});

if let Err(_) =
self.eq_types(a, b, Locations::All(span), ConstraintCategory::BoringNoLocation)
{
Expand Down
39 changes: 38 additions & 1 deletion compiler/rustc_infer/src/infer/nll_relate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::infer::{ConstVarValue, ConstVariableValue};
use crate::infer::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::traits::{Obligation, PredicateObligation};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::DefKind;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation};
Expand Down Expand Up @@ -410,7 +411,7 @@ where
let (a, b) = match (a.kind(), b.kind()) {
(&ty::Alias(ty::Opaque, ..), _) => (a, generalize(b, false)?),
(_, &ty::Alias(ty::Opaque, ..)) => (generalize(a, true)?, b),
_ => unreachable!(),
_ => unreachable!("expected an opaque, got {a} and {b}"),
};
let cause = ObligationCause::dummy_with_span(self.delegate.span());
let obligations = self
Expand Down Expand Up @@ -627,6 +628,42 @@ where
self.relate_opaques(a, b)
}

// Handle default-body RPITITs
Copy link
Member Author

@compiler-errors compiler-errors Jan 17, 2023

Choose a reason for hiding this comment

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

Within a method, relating one of that method's RPITITs against a concrete type should act just like opaque type inference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looked at this for ~10 minutes and I don't understand it '^^ would probably be best to chat about this in sync next week

(
&ty::Alias(ty::Projection, ty::AliasTy { def_id: a_def_id, .. }),
&ty::Alias(ty::Projection, ty::AliasTy { def_id: b_def_id, .. }),
) if a_def_id == b_def_id
&& self.tcx().def_kind(a_def_id) == DefKind::ImplTraitPlaceholder
&& a_def_id.as_local().map_or(false, |def_id| {
self.infcx.opaque_type_origin(def_id, self.delegate.span()).is_some()
}) =>
{
infcx.super_combine_tys(self, a, b).or_else(|err| {
self.tcx().sess.delay_span_bug(
self.delegate.span(),
"failure to relate an opaque to itself should result in an error later on",
);
if a_def_id.is_local() { self.relate_opaques(a, b) } else { Err(err) }
})
}
(&ty::Alias(ty::Projection, ty::AliasTy { def_id, substs, .. }), _)
if self.tcx().def_kind(def_id) == DefKind::ImplTraitPlaceholder
&& def_id.as_local().map_or(false, |def_id| {
self.infcx.opaque_type_origin(def_id, self.delegate.span()).is_some()
}) =>
{
self.relate_opaques(self.tcx().mk_opaque(def_id, substs), b)
}

(_, &ty::Alias(ty::Projection, ty::AliasTy { def_id, substs, .. }))
if self.tcx().def_kind(def_id) == DefKind::ImplTraitPlaceholder
&& def_id.as_local().map_or(false, |def_id| {
self.infcx.opaque_type_origin(def_id, self.delegate.span()).is_some()
}) =>
{
self.relate_opaques(a, self.tcx().mk_opaque(def_id, substs))
}

(&ty::Alias(ty::Projection, projection_ty), _)
if D::normalization() == NormalizationStrategy::Lazy =>
{
Expand Down
21 changes: 15 additions & 6 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<'tcx> InferCtxt<'tcx> {
span: Span,
param_env: ty::ParamEnv<'tcx>,
) -> InferOk<'tcx, T> {
if !value.has_opaque_types() {
if !value.has_projections() {
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to handle either RPITITs (projections) and opaques... hopefully this doesn't hurt perf too much :/

return InferOk { value, obligations: vec![] };
}
let mut obligations = vec![];
Expand All @@ -66,8 +66,11 @@ impl<'tcx> InferCtxt<'tcx> {
lt_op: |lt| lt,
ct_op: |ct| ct,
ty_op: |ty| match *ty.kind() {
ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. })
if replace_opaque_type(def_id) =>
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if matches!(
self.tcx.def_kind(def_id),
DefKind::OpaqueTy | DefKind::ImplTraitPlaceholder
) && replace_opaque_type(def_id) =>
{
let def_span = self.tcx.def_span(def_id);
let span = if span.contains(def_span) { def_span } else { span };
Expand All @@ -81,9 +84,15 @@ impl<'tcx> InferCtxt<'tcx> {
span,
});
obligations.extend(
self.handle_opaque_type(ty, ty_var, true, &cause, param_env)
.unwrap()
.obligations,
self.handle_opaque_type(
self.tcx.mk_opaque(def_id, substs),
ty_var,
true,
&cause,
param_env,
)
.unwrap()
.obligations,
);
ty_var
}
Expand Down
36 changes: 3 additions & 33 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,7 @@ enum ProjectionCandidate<'tcx> {
/// From an "impl" (or a "pseudo-impl" returned by select)
Select(Selection<'tcx>),

ImplTraitInTrait(ImplTraitInTraitCandidate<'tcx>),
}

#[derive(PartialEq, Eq, Debug)]
enum ImplTraitInTraitCandidate<'tcx> {
// The `impl Trait` from a trait function's default body
Trait,
// A concrete type provided from a trait's `impl Trait` from an impl
Impl(ImplSourceUserDefinedData<'tcx, PredicateObligation<'tcx>>),
ImplTraitInTrait(ImplSourceUserDefinedData<'tcx, PredicateObligation<'tcx>>),
}

enum ProjectionCandidateSet<'tcx> {
Expand Down Expand Up @@ -1292,17 +1284,6 @@ fn assemble_candidate_for_impl_trait_in_trait<'cx, 'tcx>(
let tcx = selcx.tcx();
if tcx.def_kind(obligation.predicate.def_id) == DefKind::ImplTraitPlaceholder {
let trait_fn_def_id = tcx.impl_trait_in_trait_parent(obligation.predicate.def_id);
// If we are trying to project an RPITIT with trait's default `Self` parameter,
// then we must be within a default trait body.
if obligation.predicate.self_ty()
== ty::InternalSubsts::identity_for_item(tcx, obligation.predicate.def_id).type_at(0)
&& tcx.associated_item(trait_fn_def_id).defaultness(tcx).has_value()
{
candidate_set.push_candidate(ProjectionCandidate::ImplTraitInTrait(
ImplTraitInTraitCandidate::Trait,
));
return;
}

let trait_def_id = tcx.parent(trait_fn_def_id);
let trait_substs =
Expand All @@ -1313,9 +1294,7 @@ fn assemble_candidate_for_impl_trait_in_trait<'cx, 'tcx>(
let _ = selcx.infcx.commit_if_ok(|_| {
match selcx.select(&obligation.with(tcx, trait_predicate)) {
Ok(Some(super::ImplSource::UserDefined(data))) => {
candidate_set.push_candidate(ProjectionCandidate::ImplTraitInTrait(
ImplTraitInTraitCandidate::Impl(data),
));
candidate_set.push_candidate(ProjectionCandidate::ImplTraitInTrait(data));
Ok(())
}
Ok(None) => {
Expand Down Expand Up @@ -1775,18 +1754,9 @@ fn confirm_candidate<'cx, 'tcx>(
ProjectionCandidate::Select(impl_source) => {
confirm_select_candidate(selcx, obligation, impl_source)
}
ProjectionCandidate::ImplTraitInTrait(ImplTraitInTraitCandidate::Impl(data)) => {
ProjectionCandidate::ImplTraitInTrait(data) => {
confirm_impl_trait_in_trait_candidate(selcx, obligation, data)
}
// If we're projecting an RPITIT for a default trait body, that's just
// the same def-id, but as an opaque type (with regular RPIT semantics).
ProjectionCandidate::ImplTraitInTrait(ImplTraitInTraitCandidate::Trait) => Progress {
term: selcx
.tcx()
.mk_opaque(obligation.predicate.def_id, obligation.predicate.substs)
.into(),
obligations: vec![],
},
};

// When checking for cycle during evaluation, we compare predicates with
Expand Down
66 changes: 66 additions & 0 deletions tests/ui/async-await/in-trait/async-default-fn-overridden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// run-pass
// edition:2021
// check-run-results

#![feature(async_fn_in_trait)]
//~^ WARN the feature `async_fn_in_trait` is incomplete and may not be safe to use

use std::future::Future;

trait AsyncTrait {
async fn default_impl() {
println!("A");
}

async fn call_default_impl() {
Self::default_impl().await
}
}

struct AsyncType;

impl AsyncTrait for AsyncType {
async fn default_impl() {
println!("B");
}
}

async fn async_main() {
AsyncType::call_default_impl().await;
}

// ------------------------------------------------------------------------- //
// Implementation Details Below...

use std::pin::Pin;
use std::task::*;

pub fn noop_waker() -> Waker {
let raw = RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE);

// SAFETY: the contracts for RawWaker and RawWakerVTable are upheld
unsafe { Waker::from_raw(raw) }
}

const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop);

unsafe fn noop_clone(_p: *const ()) -> RawWaker {
RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE)
}

unsafe fn noop(_p: *const ()) {}

fn main() {
let mut fut = async_main();

// Poll loop, just to test the future...
let waker = noop_waker();
let ctx = &mut Context::from_waker(&waker);

loop {
match unsafe { Pin::new_unchecked(&mut fut).poll(ctx) } {
Poll::Pending => {}
Poll::Ready(()) => break,
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
B
Copy link
Contributor

@lcnr lcnr Feb 8, 2023

Choose a reason for hiding this comment

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

instead of printing stuff return a bool and assert that it is correct?

11 changes: 11 additions & 0 deletions tests/ui/async-await/in-trait/async-default-fn-overridden.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
warning: the feature `async_fn_in_trait` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/async-default-fn-overridden.rs:5:12
|
LL | #![feature(async_fn_in_trait)]
| ^^^^^^^^^^^^^^^^^
|
= note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
= note: `#[warn(incomplete_features)]` on by default

warning: 1 warning emitted

3 changes: 0 additions & 3 deletions tests/ui/impl-trait/in-trait/default-body-type-err.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ error[E0271]: type mismatch resolving `<&i32 as Deref>::Target == String`
|
LL | fn lol(&self) -> impl Deref<Target = String> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found struct `String`
LL |
LL | &1i32
| ----- return type was inferred to be `&i32` here

error: aborting due to previous error

Expand Down