Skip to content

Commit

Permalink
Rollup merge of #130234 - lukas-code:ptr-cast-errors, r=WaffleLapkin
Browse files Browse the repository at this point in the history 
improve compile errors for invalid ptr-to-ptr casts with trait objects

This is a follow-up to #120248 to improve some of its error messages.

1. Make the borrowcheck error for "type annotation requires that x must outlive y" actually point at the type annotation, i.e. the type `T` in a `x as T` cast. This makes the error more consistent with other errors caused by type annotation in other places, such as
```text
error: lifetime may not live long enough
 --> src/lib.rs:4:12
  |
3 | fn bar(a: &i32) {
  |           - let's call the lifetime of this reference `'1`
4 |     let b: &'static i32 = a;
  |            ^^^^^^^^^^^^ type annotation requires that `'1` must outlive `'static`
```

2. Don't say "cast" when we actually mean "coercion" and give borrowcheck errors from actual casts (which is currently just the check added in #120248) a higher priority than ones from coercions. This can improve the errors for ptr-to-ptr cast between trait objects because they are are lowered as an upcast "unsizing" coercion if possible (which may be the identity upcast) followed by the actual cast.

3. Bring back the old "casting X as Y is invalid" message for type mismatch in the principals and reword the "vtable kinds may not match" message to more accurately describe the pointer metadata and not refer to "vtables" if the metadata is unknown.

fixes #130030

r? `@WaffleLapkin` but feel free to reassign
  • Loading branch information
@tgross35
tgross35 authored Sep 24, 2024
2 parents 363ae41 + b62e72c commit 75296fc
Show file tree
Hide file tree
Showing 122 changed files with 552 additions and 394 deletions.
8 changes: 6 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,11 @@ impl<'tcx> BorrowExplanation<'tcx> {
}
}

if let ConstraintCategory::Cast { unsize_to: Some(unsize_ty) } = category {
if let ConstraintCategory::Cast {
is_implicit_coercion: true,
unsize_to: Some(unsize_ty),
} = category
{
self.add_object_lifetime_default_note(tcx, err, unsize_ty);
}
self.add_lifetime_bound_suggestion_to_diagnostic(err, &category, span, region_name);
Expand Down Expand Up @@ -740,7 +744,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
// If we see an unsized cast, then if it is our data we should check
// whether it is being cast to a trait object.
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::Unsize),
CastKind::PointerCoercion(PointerCoercion::Unsize, _),
operand,
ty,
) => {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
ConstraintCategory::Yield => "yielding this value ",
ConstraintCategory::UseAsConst => "using this value as a constant ",
ConstraintCategory::UseAsStatic => "using this value as a static ",
ConstraintCategory::Cast { .. } => "cast ",
ConstraintCategory::Cast { is_implicit_coercion: false, .. } => "cast ",
ConstraintCategory::Cast { is_implicit_coercion: true, .. } => "coercion ",
ConstraintCategory::CallArgument(_) => "argument ",
ConstraintCategory::TypeAnnotation => "type annotation ",
ConstraintCategory::ClosureBounds => "closure body ",
Expand Down
71 changes: 44 additions & 27 deletions compiler/rustc_borrowck/src/type_check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1975,8 +1975,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
Rvalue::Cast(cast_kind, op, ty) => {
self.check_operand(op, location);

match cast_kind {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => {
match *cast_kind {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, coercion_source) => {
let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
let src_sig = op.ty(body, tcx).fn_sig(tcx);

// HACK: This shouldn't be necessary... We can remove this when we actually
Expand Down Expand Up @@ -2007,15 +2008,15 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
);

let src_ty = self.normalize(src_ty, location);
if let Err(terr) = self.sub_types(
src_ty,
*ty,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand All @@ -2036,7 +2037,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
);

// The type that we see in the fcx is like
Expand All @@ -2049,7 +2050,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
src_ty,
*ty,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand All @@ -2062,19 +2063,23 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(safety)) => {
CastKind::PointerCoercion(
PointerCoercion::ClosureFnPointer(safety),
coercion_source,
) => {
let sig = match op.ty(body, tcx).kind() {
ty::Closure(_, args) => args.as_closure().sig(),
_ => bug!(),
};
let ty_fn_ptr_from =
Ty::new_fn_ptr(tcx, tcx.signature_unclosure(sig, *safety));
Ty::new_fn_ptr(tcx, tcx.signature_unclosure(sig, safety));

let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
if let Err(terr) = self.sub_types(
ty_fn_ptr_from,
*ty,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand All @@ -2087,7 +2092,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer) => {
CastKind::PointerCoercion(
PointerCoercion::UnsafeFnPointer,
coercion_source,
) => {
let fn_sig = op.ty(body, tcx).fn_sig(tcx);

// The type that we see in the fcx is like
Expand All @@ -2099,11 +2107,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

let ty_fn_ptr_from = tcx.safe_to_unsafe_fn_ty(fn_sig);

let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
if let Err(terr) = self.sub_types(
ty_fn_ptr_from,
*ty,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand All @@ -2116,30 +2125,29 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::PointerCoercion(PointerCoercion::Unsize) => {
CastKind::PointerCoercion(PointerCoercion::Unsize, coercion_source) => {
let &ty = ty;
let trait_ref = ty::TraitRef::new(
tcx,
tcx.require_lang_item(LangItem::CoerceUnsized, Some(span)),
[op.ty(body, tcx), ty],
);

let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
let unsize_to = tcx.fold_regions(ty, |r, _| {
if let ty::ReVar(_) = r.kind() { tcx.lifetimes.re_erased } else { r }
});
self.prove_trait_ref(
trait_ref,
location.to_locations(),
ConstraintCategory::Cast {
unsize_to: Some(tcx.fold_regions(ty, |r, _| {
if let ty::ReVar(_) = r.kind() {
tcx.lifetimes.re_erased
} else {
r
}
})),
is_implicit_coercion,
unsize_to: Some(unsize_to),
},
);
}

CastKind::DynStar => {
CastKind::PointerCoercion(PointerCoercion::DynStar, coercion_source) => {
// get the constraints from the target type (`dyn* Clone`)
//
// apply them to prove that the source type `Foo` implements `Clone` etc
Expand All @@ -2150,12 +2158,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

let self_ty = op.ty(body, tcx);

let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
self.prove_predicates(
existential_predicates
.iter()
.map(|predicate| predicate.with_self_ty(tcx, self_ty)),
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
);

let outlives_predicate = tcx.mk_predicate(Binder::dummy(
Expand All @@ -2166,11 +2175,14 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.prove_predicate(
outlives_predicate,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
);
}

CastKind::PointerCoercion(PointerCoercion::MutToConstPointer) => {
CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer,
coercion_source,
) => {
let ty::RawPtr(ty_from, hir::Mutability::Mut) = op.ty(body, tcx).kind()
else {
span_mirbug!(self, rvalue, "unexpected base type for cast {:?}", ty,);
Expand All @@ -2180,11 +2192,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
span_mirbug!(self, rvalue, "unexpected target type for cast {:?}", ty,);
return;
};
let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
if let Err(terr) = self.sub_types(
*ty_from,
*ty_to,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand All @@ -2197,7 +2210,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

CastKind::PointerCoercion(PointerCoercion::ArrayToPointer) => {
CastKind::PointerCoercion(PointerCoercion::ArrayToPointer, coercion_source) => {
let ty_from = op.ty(body, tcx);

let opt_ty_elem_mut = match ty_from.kind() {
Expand Down Expand Up @@ -2242,11 +2255,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
return;
}

let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
if let Err(terr) = self.sub_types(
*ty_elem,
*ty_to,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -2427,7 +2441,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
src_obj,
dst_obj,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
ConstraintCategory::Cast {
is_implicit_coercion: false,
unsize_to: None,
},
)
.unwrap();
}
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_codegen_cranelift/src/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ fn codegen_stmt<'tcx>(
lval.write_cvalue(fx, res);
}
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer),
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _),
ref operand,
to_ty,
) => {
Expand All @@ -677,7 +677,7 @@ fn codegen_stmt<'tcx>(
}
}
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer),
CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer, _),
ref operand,
to_ty,
) => {
Expand All @@ -688,6 +688,7 @@ fn codegen_stmt<'tcx>(
Rvalue::Cast(
CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
_,
),
..,
) => {
Expand Down Expand Up @@ -741,7 +742,7 @@ fn codegen_stmt<'tcx>(
}
}
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_)),
CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_), _),
ref operand,
_to_ty,
) => {
Expand All @@ -763,14 +764,18 @@ fn codegen_stmt<'tcx>(
}
}
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::Unsize),
CastKind::PointerCoercion(PointerCoercion::Unsize, _),
ref operand,
_to_ty,
) => {
let operand = codegen_operand(fx, operand);
crate::unsize::coerce_unsized_into(fx, operand, lval);
}
Rvalue::Cast(CastKind::DynStar, ref operand, _) => {
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::DynStar, _),
ref operand,
_,
) => {
let operand = codegen_operand(fx, operand);
crate::unsize::coerce_dyn_star(fx, operand, lval);
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::Unsize),
mir::CastKind::PointerCoercion(PointerCoercion::Unsize, _),
ref source,
_,
) => {
Expand Down Expand Up @@ -465,7 +465,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let lladdr = bx.ptrtoint(llptr, llcast_ty);
OperandValue::Immediate(lladdr)
}
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => {
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _) => {
match *operand.layout.ty.kind() {
ty::FnDef(def_id, args) => {
let instance = ty::Instance::resolve_for_fn_ptr(
Expand All @@ -481,7 +481,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
_ => bug!("{} cannot be reified to a fn ptr", operand.layout.ty),
}
}
mir::CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_)) => {
mir::CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_), _) => {
match *operand.layout.ty.kind() {
ty::Closure(def_id, args) => {
let instance = Instance::resolve_closure(
Expand All @@ -496,19 +496,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
_ => bug!("{} cannot be cast to a fn ptr", operand.layout.ty),
}
}
mir::CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer) => {
mir::CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer, _) => {
// This is a no-op at the LLVM level.
operand.val
}
mir::CastKind::PointerCoercion(PointerCoercion::Unsize) => {
mir::CastKind::PointerCoercion(PointerCoercion::Unsize, _) => {
assert!(bx.cx().is_backend_scalar_pair(cast));
let (lldata, llextra) = operand.val.pointer_parts();
let (lldata, llextra) =
base::unsize_ptr(bx, lldata, operand.layout.ty, cast.ty, llextra);
OperandValue::Pair(lldata, llextra)
}
mir::CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer, _
) => {
bug!("{kind:?} is for borrowck, and should never appear in codegen");
}
Expand All @@ -526,7 +526,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bug!("unexpected non-pair operand");
}
}
mir::CastKind::DynStar => {
mir::CastKind::PointerCoercion(PointerCoercion::DynStar, _) => {
let (lldata, llextra) = operand.val.pointer_parts();
let (lldata, llextra) =
base::cast_to_dyn_star(bx, lldata, operand.layout, cast.ty, llextra);
Expand Down
Loading

0 comments on commit 75296fc

Please sign in to comment.