Skip to content

Commit

Permalink
Rollup merge of #108551 - compiler-errors:rpitit-bad-spec, r=oli-obk
Browse files Browse the repository at this point in the history
Descriptive error when users try to combine RPITIT/AFIT with specialization

Previously we failed with some esoteric error like:

```
error[E0053]: method `foo` has an incompatible type for trait
  --> $DIR/dont-project-to-specializable-projection.rs:14:35
   |
LL |     default async fn foo(_: T) -> &'static str {
   |                                   ^^^^^^^^^^^^ expected associated type, found future
   |
note: type in trait
  --> $DIR/dont-project-to-specializable-projection.rs:10:27
   |
LL |     async fn foo(_: T) -> &'static str;
   |                           ^^^^^^^^^^^^
   = note: expected signature `fn(_) -> impl Future<Output = &'static str>`
              found signature `fn(_) -> impl Future<Output = &'static str>`
```

Now we error like:

```
error: async associated function in trait cannot be specialized
  --> $DIR/dont-project-to-specializable-projection.rs:14:5
   |
LL |     default async fn foo(_: T) -> &'static str {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: specialization behaves in inconsistent and surprising ways with `#![feature(async_fn_in_trait)]`, and for now is disallowed
```
  • Loading branch information
matthiaskrgr authored Mar 1, 2023
2 parents 78f9bb1 + ecac8fd commit b2dc8c5
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 65 deletions.
40 changes: 36 additions & 4 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,17 +792,19 @@ fn check_impl_items_against_trait<'tcx>(
trait_def.must_implement_one_of.as_deref();

for &trait_item_id in tcx.associated_item_def_ids(impl_trait_ref.def_id) {
let is_implemented = ancestors
.leaf_def(tcx, trait_item_id)
let leaf_def = ancestors.leaf_def(tcx, trait_item_id);

let is_implemented = leaf_def
.as_ref()
.map_or(false, |node_item| node_item.item.defaultness(tcx).has_value());

if !is_implemented && tcx.impl_defaultness(impl_id).is_final() {
missing_items.push(tcx.associated_item(trait_item_id));
}

// true if this item is specifically implemented in this impl
let is_implemented_here = ancestors
.leaf_def(tcx, trait_item_id)
let is_implemented_here = leaf_def
.as_ref()
.map_or(false, |node_item| !node_item.defining_node.is_from_trait());

if !is_implemented_here {
Expand Down Expand Up @@ -831,6 +833,36 @@ fn check_impl_items_against_trait<'tcx>(
}
}
}

if let Some(leaf_def) = &leaf_def
&& !leaf_def.is_final()
&& let def_id = leaf_def.item.def_id
&& tcx.impl_method_has_trait_impl_trait_tys(def_id)
{
let def_kind = tcx.def_kind(def_id);
let descr = tcx.def_kind_descr(def_kind, def_id);
let (msg, feature) = if tcx.asyncness(def_id).is_async() {
(
format!("async {descr} in trait cannot be specialized"),
sym::async_fn_in_trait,
)
} else {
(
format!(
"{descr} with return-position `impl Trait` in trait cannot be specialized"
),
sym::return_position_impl_trait_in_trait,
)
};
tcx.sess
.struct_span_err(tcx.def_span(def_id), msg)
.note(format!(
"specialization behaves in inconsistent and \
surprising ways with `#![feature({feature})]`, \
and for now is disallowed"
))
.emit();
}
}

if !missing_items.is_empty() {
Expand Down
30 changes: 1 addition & 29 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,34 +1101,6 @@ fn should_encode_const(def_kind: DefKind) -> bool {
}
}

fn should_encode_trait_impl_trait_tys(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
if tcx.def_kind(def_id) != DefKind::AssocFn {
return false;
}

let Some(item) = tcx.opt_associated_item(def_id) else { return false; };
if item.container != ty::AssocItemContainer::ImplContainer {
return false;
}

let Some(trait_item_def_id) = item.trait_item_def_id else { return false; };

// FIXME(RPITIT): This does a somewhat manual walk through the signature
// of the trait fn to look for any RPITITs, but that's kinda doing a lot
// of work. We can probably remove this when we refactor RPITITs to be
// associated types.
tcx.fn_sig(trait_item_def_id).subst_identity().skip_binder().output().walk().any(|arg| {
if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Alias(ty::Projection, data) = ty.kind()
&& tcx.def_kind(data.def_id) == DefKind::ImplTraitPlaceholder
{
true
} else {
false
}
})
}

// Return `false` to avoid encoding impl trait in trait, while we don't use the query.
fn should_encode_fn_impl_trait_in_trait<'tcx>(_tcx: TyCtxt<'tcx>, _def_id: DefId) -> bool {
false
Expand Down Expand Up @@ -1211,7 +1183,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if let DefKind::Enum | DefKind::Struct | DefKind::Union = def_kind {
self.encode_info_for_adt(def_id);
}
if should_encode_trait_impl_trait_tys(tcx, def_id)
if tcx.impl_method_has_trait_impl_trait_tys(def_id)
&& let Ok(table) = self.tcx.collect_return_position_impl_trait_in_trait_tys(def_id)
{
record!(self.tables.trait_impl_trait_tys[def_id] <- table);
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2541,6 +2541,34 @@ impl<'tcx> TyCtxt<'tcx> {
}
def_id
}

pub fn impl_method_has_trait_impl_trait_tys(self, def_id: DefId) -> bool {
if self.def_kind(def_id) != DefKind::AssocFn {
return false;
}

let Some(item) = self.opt_associated_item(def_id) else { return false; };
if item.container != ty::AssocItemContainer::ImplContainer {
return false;
}

let Some(trait_item_def_id) = item.trait_item_def_id else { return false; };

// FIXME(RPITIT): This does a somewhat manual walk through the signature
// of the trait fn to look for any RPITITs, but that's kinda doing a lot
// of work. We can probably remove this when we refactor RPITITs to be
// associated types.
self.fn_sig(trait_item_def_id).subst_identity().skip_binder().output().walk().any(|arg| {
if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Alias(ty::Projection, data) = ty.kind()
&& self.def_kind(data.def_id) == DefKind::ImplTraitPlaceholder
{
true
} else {
false
}
})
}
}

/// Yields the parent function's `LocalDefId` if `def_id` is an `impl Trait` definition.
Expand Down
24 changes: 4 additions & 20 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1307,25 +1307,8 @@ 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))) => {
let Ok(leaf_def) = specialization_graph::assoc_def(tcx, data.impl_def_id, trait_fn_def_id) else {
return Err(());
};
// Only reveal a specializable default if we're past type-checking
// and the obligation is monomorphic, otherwise passes such as
// transmute checking and polymorphic MIR optimizations could
// get a result which isn't correct for all monomorphizations.
if leaf_def.is_final()
|| (obligation.param_env.reveal() == Reveal::All
&& !selcx
.infcx
.resolve_vars_if_possible(obligation.predicate.trait_ref(tcx))
.still_further_specializable())
{
candidate_set.push_candidate(ProjectionCandidate::ImplTraitInTrait(data));
Ok(())
} else {
Err(())
}
candidate_set.push_candidate(ProjectionCandidate::ImplTraitInTrait(data));
Ok(())
}
Ok(None) => {
candidate_set.mark_ambiguous();
Expand Down Expand Up @@ -2216,7 +2199,8 @@ fn confirm_impl_trait_in_trait_candidate<'tcx>(
Ok(assoc_ty) => assoc_ty,
Err(guar) => return Progress::error(tcx, guar),
};
if !leaf_def.item.defaultness(tcx).has_value() {
// We don't support specialization for RPITITs anyways... yet.
if !leaf_def.is_final() {
return Progress { term: tcx.ty_error_misc().into(), obligations };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,13 @@ 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

error[E0053]: method `foo` has an incompatible type for trait
--> $DIR/dont-project-to-specializable-projection.rs:14:35
error: async associated function in trait cannot be specialized
--> $DIR/dont-project-to-specializable-projection.rs:14:5
|
LL | default async fn foo(_: T) -> &'static str {
| ^^^^^^^^^^^^ expected associated type, found future
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type in trait
--> $DIR/dont-project-to-specializable-projection.rs:10:27
|
LL | async fn foo(_: T) -> &'static str;
| ^^^^^^^^^^^^
= note: expected signature `fn(_) -> impl Future<Output = &'static str>`
found signature `fn(_) -> impl Future<Output = &'static str>`
= note: specialization behaves in inconsistent and surprising ways with `#![feature(async_fn_in_trait)]`, and for now is disallowed

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0053`.
1 change: 1 addition & 0 deletions tests/ui/impl-trait/in-trait/specialization-broken.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ where
{
fn bar(&self) -> U {
//~^ ERROR method `bar` has an incompatible type for trait
//~| ERROR method with return-position `impl Trait` in trait cannot be specialized
*self
}
}
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/impl-trait/in-trait/specialization-broken.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ LL | fn bar(&self) -> impl Sized;
= note: expected signature `fn(&U) -> impl Sized`
found signature `fn(&U) -> U`

error: aborting due to previous error
error: method with return-position `impl Trait` in trait cannot be specialized
--> $DIR/specialization-broken.rs:16:5
|
LL | fn bar(&self) -> U {
| ^^^^^^^^^^^^^^^^^^
|
= note: specialization behaves in inconsistent and surprising ways with `#![feature(return_position_impl_trait_in_trait)]`, and for now is disallowed

error: aborting due to 2 previous errors

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

0 comments on commit b2dc8c5

Please sign in to comment.