Skip to content

Commit

Permalink
wf: ensure that non-Rust-functions have sized arguments, and everyone…
Browse files Browse the repository at this point in the history
… has sized return types
  • Loading branch information
RalfJung committed Oct 29, 2023
1 parent bbcc169 commit c6a7fb1
Show file tree
Hide file tree
Showing 44 changed files with 416 additions and 201 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,7 @@ fn check_fn_or_method<'tcx>(
let has_implicit_self = hir_decl.implicit_self != hir::ImplicitSelfKind::None;
let mut inputs = sig.inputs().iter().skip(if has_implicit_self { 1 } else { 0 });
// Check that the argument is a tuple and is sized
// FIXME: move this to WF check? Currently it is duplicated here and in `confirm_builtin_call` in callee.rs.
if let Some(ty) = inputs.next() {
wfcx.register_bound(
ObligationCause::new(span, wfcx.body_def_id, ObligationCauseCode::RustCall),
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/src/hir_wf_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ pub fn provide(providers: &mut Providers) {
*providers = Providers { diagnostic_hir_wf_check, ..*providers };
}

/// HIR-based well-formedness check, for diagnostics only.
/// This is run after there was a WF error, to try get a better message pointing out what went wrong
/// here.
// Ideally, this would be in `rustc_trait_selection`, but we
// need access to `ItemCtxt`
fn diagnostic_hir_wf_check<'tcx>(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);

if fn_sig.abi == abi::Abi::RustCall {
// FIXME: move this to WF check? Currently it is duplicated here and in `check_fn_or_method` in wfcheck.rs.
let sp = arg_exprs.last().map_or(call_expr.span, |expr| expr.span);
if let Some(ty) = fn_sig.inputs().last().copied() {
self.register_bound(
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ pub(super) fn check_fn<'a, 'tcx>(
fcx.check_pat_top(&param.pat, param_ty, ty_span, None, None);

// Check that argument is Sized.
if !params_can_be_unsized {
// FIXME: can we share this (and the return type check below) with WF-checking on function
// signatures? However, here we have much better spans available than if we fire an
// obligation for our signature to be well-formed.
if !params_can_be_unsized || !fn_sig.abi.supports_unsized_args() {
fcx.require_type_is_sized(
param_ty,
param.pat.span,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.resolve_lang_item_path(lang_item, expr.span, expr.hir_id, hir_id).1
}

/// Called for any way that a path is mentioned in an expression.
/// If the path is used in a function call, `args` has the arguments, otherwise it is empty.
pub(crate) fn check_expr_path(
&self,
qpath: &'tcx hir::QPath<'tcx>,
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_target/src/abi/call/x86_64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ fn reg_component(cls: &[Option<Class>], i: &mut usize, size: Size) -> Option<Reg
}
}

fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
let mut i = 0;
let lo = reg_component(cls, &mut i, size)?;
let lo = reg_component(cls, &mut i, size).unwrap();
let offset = Size::from_bytes(8) * (i as u64);
let mut target = CastTarget::from(lo);
if size > offset {
Expand All @@ -164,7 +164,7 @@ fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
}
}
assert_eq!(reg_component(cls, &mut i, Size::ZERO), None);
Some(target)
target
}

const MAX_INT_REGS: usize = 6; // RDI, RSI, RDX, RCX, R8, R9
Expand Down Expand Up @@ -227,9 +227,7 @@ where
// split into sized chunks passed individually
if arg.layout.is_aggregate() {
let size = arg.layout.size;
if let Some(cast_target) = cast_target(cls, size) {
arg.cast_to(cast_target);
}
arg.cast_to(cast_target(cls, size));
} else {
arg.extend_integer_width_to(32);
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_target/src/spec/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ impl Abi {
_ => false,
}
}

/// Whether this ABI can in principle support unsized arguments.
/// There might be further restrictions such as nightly feature flags!
pub fn supports_unsized_args(self) -> bool {
match self {
Self::Rust | Self::RustCall | Self::RustIntrinsic | Self::PlatformIntrinsic => true,
_ => false,
}
}
}

#[derive(Copy, Clone)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
);
}

let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(obligation.cause.body_id));
let Some(body) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
return false;
};
let body = self.tcx.hir().body(body);

let mut visitor = ReturnsVisitor::default();
visitor.visit_body(&body);
Expand Down Expand Up @@ -1922,15 +1925,19 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// Point at all the `return`s in the function as they have failed trait bounds.
let mut visitor = ReturnsVisitor::default();
visitor.visit_body(&body);
let typeck_results = self.typeck_results.as_ref().unwrap();
for expr in &visitor.returns {
if let Some(returned_ty) = typeck_results.node_type_opt(expr.hir_id) {
let ty = self.resolve_vars_if_possible(returned_ty);
if ty.references_error() {
// don't print out the [type error] here
err.delay_as_bug();
} else {
err.span_label(expr.span, format!("this returned value is of type `{ty}`"));
if let Some(typeck_results) = self.typeck_results.as_ref() {
for expr in &visitor.returns {
if let Some(returned_ty) = typeck_results.node_type_opt(expr.hir_id) {
let ty = self.resolve_vars_if_possible(returned_ty);
if ty.references_error() {
// don't print out the [type error] here
err.delay_as_bug();
} else {
err.span_label(
expr.span,
format!("this returned value is of type `{ty}`"),
);
}
}
}
}
Expand Down
23 changes: 20 additions & 3 deletions compiler/rustc_trait_selection/src/traits/wf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,10 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
self.out.extend(obligations);
}

ty::FnPtr(_) => {
// let the loop iterate into the argument/return
// types appearing in the fn signature
ty::FnPtr(fn_sig) => {
// The loop iterates into the argument/return types appearing in the fn
// signature, but we need to do some extra checks.
self.compute_fn_sig_obligations(fn_sig)
}

ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
Expand Down Expand Up @@ -806,6 +807,22 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
}
}

/// Add the obligations for this signature to be well-formed to `out`.
fn compute_fn_sig_obligations(&mut self, sig: ty::PolyFnSig<'tcx>) {
// The return type must always be sized.
// FIXME(RalfJung): is skip_binder right? It's what the type walker used in `compute` also does.
self.require_sized(sig.skip_binder().output(), traits::SizedReturnType);
// For non-Rust ABIs, the argument type must always be sized.
// FIXME(RalfJung): we don't do the Rust ABI check here, since that depends on feature gates
// and it's not clear to me whether WF depending on feature gates (which can differ across
// crates) is possible or not.
if !sig.skip_binder().abi.supports_unsized_args() {
for &arg in sig.skip_binder().inputs() {
self.require_sized(arg, traits::SizedArgumentType(None));
}
}
}

#[instrument(level = "debug", skip(self))]
fn nominal_obligations(
&mut self,
Expand Down
21 changes: 16 additions & 5 deletions tests/ui/abi/compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,19 +307,30 @@ mod arrays {
}

// Some tests with unsized types (not all wrappers are compatible with that).
macro_rules! assert_abi_compatible_unsized {
($name:ident, $t1:ty, $t2:ty) => {
mod $name {
use super::*;
// Declaring a `type` doesn't even check well-formedness, so we also declare a function.
fn check_wf(_x: $t1, _y: $t2) {}
// Can only test arguments and only the Rust ABI, since it's unsized.
#[rustc_abi(assert_eq)]
type TestRust = (fn($t1), fn($t2));
}
};
}
macro_rules! test_transparent_unsized {
($name:ident, $t:ty) => {
mod $name {
use super::*;
assert_abi_compatible!(wrap1, $t, Wrapper1<$t>);
assert_abi_compatible!(wrap1_reprc, ReprC1<$t>, ReprC1<Wrapper1<$t>>);
assert_abi_compatible!(wrap2, $t, Wrapper2<$t>);
assert_abi_compatible!(wrap2_reprc, ReprC1<$t>, ReprC1<Wrapper2<$t>>);
assert_abi_compatible_unsized!(wrap1, $t, Wrapper1<$t>);
assert_abi_compatible_unsized!(wrap1_reprc, ReprC1<$t>, ReprC1<Wrapper1<$t>>);
assert_abi_compatible_unsized!(wrap2, $t, Wrapper2<$t>);
assert_abi_compatible_unsized!(wrap2_reprc, ReprC1<$t>, ReprC1<Wrapper2<$t>>);
}
};
}

#[cfg(not(any(target_arch = "mips64", target_arch = "sparc64")))]
mod unsized_ {
use super::*;
test_transparent_unsized!(str_, str);
Expand Down
8 changes: 0 additions & 8 deletions tests/ui/abi/issue-94223.rs

This file was deleted.

14 changes: 0 additions & 14 deletions tests/ui/associated-types/associated-types-unsized.fixed

This file was deleted.

2 changes: 1 addition & 1 deletion tests/ui/associated-types/associated-types-unsized.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// run-rustfix
#![allow(dead_code, unused_variables)]

trait Get {
Expand All @@ -8,6 +7,7 @@ trait Get {

fn foo<T:Get>(t: T) {
let x = t.get(); //~ ERROR the size for values of type
//~| ERROR the size for values of type
}

fn main() {
Expand Down
17 changes: 15 additions & 2 deletions tests/ui/associated-types/associated-types-unsized.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
error[E0277]: the size for values of type `<T as Get>::Value` cannot be known at compilation time
--> $DIR/associated-types-unsized.rs:10:9
--> $DIR/associated-types-unsized.rs:9:15
|
LL | let x = t.get();
| ^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `<T as Get>::Value`
= note: the return type of a function must have a statically known size
help: consider further restricting the associated type
|
LL | fn foo<T:Get>(t: T) where <T as Get>::Value: Sized {
| ++++++++++++++++++++++++++++++

error[E0277]: the size for values of type `<T as Get>::Value` cannot be known at compilation time
--> $DIR/associated-types-unsized.rs:9:9
|
LL | let x = t.get();
| ^ doesn't have a size known at compile-time
Expand All @@ -12,6 +25,6 @@ help: consider further restricting the associated type
LL | fn foo<T:Get>(t: T) where <T as Get>::Value: Sized {
| ++++++++++++++++++++++++++++++

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
6 changes: 3 additions & 3 deletions tests/ui/closures/closure-return-type-must-be-sized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,21 @@ impl A for Box<dyn A> {

fn main() {
a::foo::<fn() -> dyn A>(); //~ ERROR E0277
a::bar::<fn() -> dyn A, _>(); //~ ERROR E0277
a::bar::<fn() -> dyn A, _>(); //~ ERROR cannot have an unboxed trait object
a::baz::<fn() -> dyn A>(); //~ ERROR E0277
a::foo::<fn() -> Box<dyn A>>(); // ok
a::bar::<fn() -> Box<dyn A>, _>(); // ok
a::baz::<fn() -> Box<dyn A>>(); // ok

b::foo::<fn() -> dyn A>(); //~ ERROR E0277
b::bar::<fn() -> dyn A, _>(); //~ ERROR E0277
b::bar::<fn() -> dyn A, _>(); //~ ERROR cannot have an unboxed trait object
b::baz::<fn() -> dyn A>(); //~ ERROR E0277
b::foo::<fn() -> Box<dyn A>>(); // ok
b::bar::<fn() -> Box<dyn A>, _>(); // ok
b::baz::<fn() -> Box<dyn A>>(); // ok

c::foo::<fn() -> dyn A>(); //~ ERROR E0277
c::bar::<fn() -> dyn A, _>(); //~ ERROR E0277
c::bar::<fn() -> dyn A, _>(); //~ ERROR cannot have an unboxed trait object
c::baz::<fn() -> dyn A>(); //~ ERROR E0277
c::foo::<fn() -> Box<dyn A>>(); // ok
c::bar::<fn() -> Box<dyn A>, _>(); // ok
Expand Down
36 changes: 14 additions & 22 deletions tests/ui/closures/closure-return-type-must-be-sized.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ LL | a::foo::<fn() -> dyn A>();
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
error[E0746]: return type cannot have an unboxed trait object
--> $DIR/closure-return-type-must-be-sized.rs:55:14
|
LL | a::bar::<fn() -> dyn A, _>();
| ^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`
note: required by a bound in `a::bar`
--> $DIR/closure-return-type-must-be-sized.rs:14:19
help: box the return type, and wrap all of the returned values in `Box::new`
|
LL | pub fn bar<F: FnOnce() -> R, R: ?Sized>() {}
| ^^^^^^^^^^^^^ required by this bound in `bar`
LL | a::bar::<Box<fn() -> dyn A>, _>();
| ++++ +

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
--> $DIR/closure-return-type-must-be-sized.rs:56:5
Expand All @@ -39,19 +36,16 @@ LL | b::foo::<fn() -> dyn A>();
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
error[E0746]: return type cannot have an unboxed trait object
--> $DIR/closure-return-type-must-be-sized.rs:62:14
|
LL | b::bar::<fn() -> dyn A, _>();
| ^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`
note: required by a bound in `b::bar`
--> $DIR/closure-return-type-must-be-sized.rs:28:19
help: box the return type, and wrap all of the returned values in `Box::new`
|
LL | pub fn bar<F: Fn() -> R, R: ?Sized>() {}
| ^^^^^^^^^ required by this bound in `bar`
LL | b::bar::<Box<fn() -> dyn A>, _>();
| ++++ +

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
--> $DIR/closure-return-type-must-be-sized.rs:63:5
Expand All @@ -71,19 +65,16 @@ LL | c::foo::<fn() -> dyn A>();
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
error[E0746]: return type cannot have an unboxed trait object
--> $DIR/closure-return-type-must-be-sized.rs:69:14
|
LL | c::bar::<fn() -> dyn A, _>();
| ^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: within `fn() -> dyn A`, the trait `Sized` is not implemented for `dyn A`
= note: required because it appears within the type `fn() -> dyn A`
note: required by a bound in `c::bar`
--> $DIR/closure-return-type-must-be-sized.rs:42:19
help: box the return type, and wrap all of the returned values in `Box::new`
|
LL | pub fn bar<F: FnMut() -> R, R: ?Sized>() {}
| ^^^^^^^^^^^^ required by this bound in `bar`
LL | c::bar::<Box<fn() -> dyn A>, _>();
| ++++ +

error[E0277]: the size for values of type `dyn A` cannot be known at compilation time
--> $DIR/closure-return-type-must-be-sized.rs:70:5
Expand All @@ -96,4 +87,5 @@ LL | c::baz::<fn() -> dyn A>();

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0277`.
Some errors have detailed explanations: E0277, E0746.
For more information about an error, try `rustc --explain E0277`.
2 changes: 1 addition & 1 deletion tests/ui/did_you_mean/E0178.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ struct Bar<'a> {
w: &'a Foo + Copy, //~ ERROR expected a path
x: &'a Foo + 'a, //~ ERROR expected a path
y: &'a mut Foo + 'a, //~ ERROR expected a path
z: fn() -> Foo + 'a, //~ ERROR expected a path
z: fn() -> &'static Foo + 'a, //~ ERROR expected a path
}

fn main() {
Expand Down
Loading

0 comments on commit c6a7fb1

Please sign in to comment.