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

[experiment] Delay coercion error rather than emitting error in coerce_unsized #138438

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
57 changes: 43 additions & 14 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use rustc_ast::util::parser::ExprPrecedence;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, Diag, ErrorGuaranteed};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, ExprKind};
use rustc_infer::infer::DefineOpaqueTypes;
use rustc_macros::{TypeFoldable, TypeVisitable};
Expand Down Expand Up @@ -155,7 +156,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

#[derive(Copy, Clone, Debug)]
#[derive(Debug)]
enum CastError<'tcx> {
ErrorGuaranteed(ErrorGuaranteed),

Expand All @@ -182,6 +183,7 @@ enum CastError<'tcx> {
/// when we're typechecking a type parameter with a ?Sized bound.
IntToWideCast(Option<&'static str>),
ForeignNonExhaustiveAdt,
PtrPtrAddingAutoTrait(Vec<DefId>),
}

impl From<ErrorGuaranteed> for CastError<'_> {
Expand Down Expand Up @@ -351,6 +353,14 @@ impl<'a, 'tcx> CastCheck<'tcx> {
PointerKind::Thin,
)
| (PointerKind::Length, PointerKind::Length) => {
if let Some(_guar) = fcx.emit_specialized_coerce_unsize_error(
self.expr.span,
self.expr_ty,
self.cast_ty,
) {
return;
}

span_bug!(self.span, "unexpected cast error: {e:?}")
}
}
Expand Down Expand Up @@ -399,6 +409,14 @@ impl<'a, 'tcx> CastCheck<'tcx> {
err.emit();
}
CastError::NonScalar => {
if let Some(_guar) = fcx.emit_specialized_coerce_unsize_error(
self.expr.span,
self.expr_ty,
self.cast_ty,
) {
return;
}

let mut err = type_error_struct!(
fcx.dcx(),
self.span,
Expand Down Expand Up @@ -548,6 +566,14 @@ impl<'a, 'tcx> CastCheck<'tcx> {
err.emit();
}
CastError::SizedUnsizedCast => {
if let Some(_guar) = fcx.emit_specialized_coerce_unsize_error(
self.expr.span,
self.expr_ty,
self.cast_ty,
) {
return;
}

let cast_ty = fcx.resolve_vars_if_possible(self.cast_ty);
let expr_ty = fcx.resolve_vars_if_possible(self.expr_ty);
fcx.dcx().emit_err(errors::CastThinPointerToWidePointer {
Expand Down Expand Up @@ -596,6 +622,21 @@ impl<'a, 'tcx> CastCheck<'tcx> {
.with_note("cannot cast an enum with a non-exhaustive variant when it's defined in another crate")
.emit();
}
CastError::PtrPtrAddingAutoTrait(added) => {
fcx.dcx().emit_err(errors::PtrCastAddAutoToObject {
span: self.span,
traits_len: added.len(),
traits: {
let mut traits: Vec<_> = added
.into_iter()
.map(|trait_did| fcx.tcx.def_path_str(trait_did))
.collect();

traits.sort();
traits.into()
},
});
}
}
}

Expand Down Expand Up @@ -940,19 +981,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
.collect::<Vec<_>>();

if !added.is_empty() {
tcx.dcx().emit_err(errors::PtrCastAddAutoToObject {
span: self.span,
traits_len: added.len(),
traits: {
let mut traits: Vec<_> = added
.into_iter()
.map(|trait_did| tcx.def_path_str(trait_did))
.collect();

traits.sort();
traits.into()
},
});
return Err(CastError::PtrPtrAddingAutoTrait(added));
}

Ok(CastKind::PtrPtrCast)
Expand Down
122 changes: 60 additions & 62 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,20 @@ use rustc_infer::infer::relate::RelateResult;
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
use rustc_infer::traits::{
IfExpressionCause, MatchExpressionArmCause, Obligation, PredicateObligation,
PredicateObligations,
PredicateObligations, SelectionError,
};
use rustc_middle::span_bug;
use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{self, AliasTy, GenericArgsRef, Ty, TyCtxt};
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Span};
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt};
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, ErrorGuaranteed, Span};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{
self, NormalizeExt, ObligationCause, ObligationCauseCode, ObligationCtxt,
self, FulfillmentErrorCode, NormalizeExt, ObligationCause, ObligationCauseCode, ObligationCtxt,
};
use smallvec::{SmallVec, smallvec};
use tracing::{debug, instrument};
Expand Down Expand Up @@ -600,55 +600,8 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
ty::TraitRef::new(self.tcx, coerce_unsized_did, [coerce_source, coerce_target]),
);

// If the root `Source: CoerceUnsized<Target>` obligation can't possibly hold,
// we don't have to assume that this is unsizing coercion (it will always lead to an error)
//
// However, we don't want to bail early all the time, since the unholdable obligations
// may be interesting for diagnostics (such as trying to coerce `&T` to `&dyn Id<This = U>`),
// so we only bail if there (likely) is another way to convert the types.
if !self.infcx.predicate_may_hold(&root_obligation) {
if let Some(dyn_metadata_adt_def_id) = self.tcx.lang_items().get(LangItem::DynMetadata)
&& let Some(metadata_type_def_id) = self.tcx.lang_items().get(LangItem::Metadata)
{
self.probe(|_| {
let ocx = ObligationCtxt::new(&self.infcx);

// returns `true` if `<ty as Pointee>::Metadata` is `DynMetadata<_>`
let has_dyn_trait_metadata = |ty| {
let metadata_ty: Result<_, _> = ocx.structurally_normalize_ty(
&ObligationCause::dummy(),
self.fcx.param_env,
Ty::new_alias(
self.tcx,
ty::AliasTyKind::Projection,
AliasTy::new(self.tcx, metadata_type_def_id, [ty]),
),
);

metadata_ty.is_ok_and(|metadata_ty| {
metadata_ty
.ty_adt_def()
.is_some_and(|d| d.did() == dyn_metadata_adt_def_id)
})
};

// If both types are raw pointers to a (wrapper over a) trait object,
// this might be a cast like `*const W<dyn Trait> -> *const dyn Trait`.
// So it's better to bail and try that. (even if the cast is not possible, for
// example due to vtables not matching, cast diagnostic will likely still be better)
//
// N.B. use `target`, not `coerce_target` (the latter is a var)
if let &ty::RawPtr(source_pointee, _) = coerce_source.kind()
&& let &ty::RawPtr(target_pointee, _) = target.kind()
&& has_dyn_trait_metadata(source_pointee)
&& has_dyn_trait_metadata(target_pointee)
{
return Err(TypeError::Mismatch);
}

Ok(())
})?;
}
return Err(TypeError::Mismatch);
}

// Use a FIFO queue for this custom fulfillment procedure.
Expand Down Expand Up @@ -725,17 +678,10 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return Err(TypeError::Mismatch);
}

// Dyn-compatibility violations or miscellaneous.
// Should have been filtered out by the `predicate_may_hold` check.
Err(err) => {
let guar = self.err_ctxt().report_selection_error(
obligation.clone(),
&obligation,
&err,
);
self.fcx.set_tainted_by_errors(guar);
// Treat this like an obligation and follow through
// with the unsizing - the lack of a coercion should
// be silent, as it causes a type mismatch later.
debug!("coerce_unsized: early return - selection error: {err:?}");
return Err(TypeError::Mismatch);
}

Ok(Some(impl_source)) => queue.extend(impl_source.nested_obligations()),
Expand Down Expand Up @@ -1120,6 +1066,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
}

pub(crate) fn emit_specialized_coerce_unsize_error(
&self,
span: Span,
source: Ty<'tcx>,
target: Ty<'tcx>,
) -> Option<ErrorGuaranteed> {
let ocx = ObligationCtxt::new_with_diagnostics(self);
let coerce_unsized_def_id = self.tcx.require_lang_item(LangItem::CoerceUnsized, Some(span));
let unsize_def_id = self.tcx.require_lang_item(LangItem::Unsize, Some(span));
ocx.register_obligation(Obligation::new(
self.tcx,
self.cause(span, ObligationCauseCode::Coercion { source, target }),
self.param_env,
ty::TraitRef::new(self.tcx, coerce_unsized_def_id, [source, target]),
));

let mut errors = ocx.select_where_possible();
// Retain the errors that don't mention, but also as a HACK we will adjust their
// root obligation, too. This is a nasty hack to preserve diagnostic parity that
// should probably be fixed by emitting better errors for failed `CoerceUnsized`.
errors.retain_mut(|err| {
if matches!(
err.code,
FulfillmentErrorCode::Select(SelectionError::TraitDynIncompatible(_)),
) || err.obligation.predicate.as_trait_clause().is_none_or(|trait_clause| {
trait_clause.def_id() != coerce_unsized_def_id
&& trait_clause.def_id() != unsize_def_id
}) {
err.root_obligation = err.obligation.clone();
true
} else {
false
}
});

if errors.is_empty() {
None
} else {
let guar = self.err_ctxt().report_fulfillment_errors(errors);
self.set_tainted_by_errors(guar);
Some(guar)
}
}

/// Probe whether `expr_ty` can be coerced to `target_ty`. This has no side-effects,
/// and may return false positives if types are not yet fully constrained by inference.
///
Expand Down Expand Up @@ -1666,6 +1656,14 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}
}
Err(coercion_error) => {
if let Some(_guar) = fcx.emit_specialized_coerce_unsize_error(
cause.span,
expression_ty,
self.merged_ty(),
) {
return;
}

// Mark that we've failed to coerce the types here to suppress
// any superfluous errors we might encounter while trying to
// emit or provide suggestions on how to fix the initial error.
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Err(e) => e,
};

if let Some(guar) =
self.emit_specialized_coerce_unsize_error(expr.span, checked_ty, expected)
{
return Ok(Ty::new_error(self.tcx, guar));
}

self.adjust_expr_for_assert_eq_macro(&mut expr, &mut expected_ty_expr);

self.set_tainted_by_errors(self.dcx().span_delayed_bug(
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
true
});

// We need to handle type mismatches due to unsizing failures specially.
errors.retain(|err| {
if let Error::Invalid(
provided_idx,
expected_idx,
Compatibility::Incompatible(Some(_)),
) = *err
{
let (_, expected_ty) = formal_and_expected_inputs[expected_idx];
let (provided_ty, provided_arg_span) = provided_arg_tys[provided_idx];

if let Some(guar) = self.emit_specialized_coerce_unsize_error(
provided_arg_span,
provided_ty,
expected_ty,
) {
reported = Some(guar);
return false;
}
}

true
});

// We're done if we found errors, but we already emitted them.
if let Some(reported) = reported
&& errors.is_empty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&obligation.cause,
);

let pointer_like_goal = normalize_with_depth_to(
self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth,
pointer_like_goal,
&mut nested,
);

nested.push(predicate_to_obligation(pointer_like_goal.upcast(tcx)));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ LL | async {
LL | break 0u8;
| ^^^^^^^^^ cannot `break` inside `async` block

error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:26:39
|
LL | let _: &dyn Future<Output = ()> = &block;
| ^^^^^^ expected `()`, found `u8`
|
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to `&dyn Future<Output = ()>`

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:21:58
|
Expand All @@ -26,13 +34,13 @@ LL | | return 0u8;
LL | | }
| |_^ expected `u8`, found `()`

error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:26:39
error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:17:39
|
LL | let _: &dyn Future<Output = ()> = &block;
| ^^^^^^ expected `()`, found `u8`
|
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to `&dyn Future<Output = ()>`
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to `&dyn Future<Output = ()>`

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:12:43
Expand All @@ -42,14 +50,6 @@ LL | fn return_targets_async_block_not_fn() -> u8 {
| |
| implicitly returns `()` as its body has no tail or `return` expression

error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to be a future that resolves to `()`, but it resolves to `u8`
--> $DIR/async-block-control-flow-static-semantics.rs:17:39
|
LL | let _: &dyn Future<Output = ()> = &block;
| ^^^^^^ expected `()`, found `u8`
|
= note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to `&dyn Future<Output = ()>`

error[E0308]: mismatched types
--> $DIR/async-block-control-flow-static-semantics.rs:49:44
|
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/cross/cross-borrow-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ LL | let _y: &dyn Trait = x;
|
= note: expected reference `&dyn Trait`
found struct `Box<dyn Trait>`
help: consider borrowing here
|
LL | let _y: &dyn Trait = &x;
| +

error: aborting due to 1 previous error

Expand Down
Loading
Loading