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

Descriptive error when users try to combine RPITIT/AFIT with specialization #108551

Merged
merged 1 commit into from
Mar 1, 2023
Merged
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
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`
Copy link
Member Author

@compiler-errors compiler-errors Feb 28, 2023

Choose a reason for hiding this comment

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

This fails to be replaced by the other message because we're not actually even assembling a ProjectionCandidate::ImplTraitInTrait here. Oh well, not that big of a deal.

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`.