Skip to content

Commit

Permalink
Auto merge of rust-lang#123531 - compiler-errors:closure-wf, r=oli-obk
Browse files Browse the repository at this point in the history
Enforce closure args + return type are WF

I found this out when investigating rust-lang#123461 (comment). Turns out we don't register WF obligations for closure args and return types, leading to the ICE.

~~I think this is a useful thing to check for, but I'd like to check what the fallout is.~~ crater is complete.

~~Worst case, I think we should enforce this across an edition boundary (and possibly eventually migrate this for all editions) -- this should be super easy to do, since this is a check in HIR wfcheck, so it can be made edition dependent.~~ I believe the regressions are manageable enough to not necessitate edition-specific behavior.

Fixes rust-lang#123461
  • Loading branch information
bors committed Apr 25, 2024
2 parents 6a9758d + 870ed4b commit 9e6c4fd
Show file tree
Hide file tree
Showing 16 changed files with 126 additions and 44 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ struct DiagCtxtInner {
}

/// A key denoting where from a diagnostic was stashed.
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub enum StashKey {
ItemNoType,
UnderscoreForArrayLengths,
Expand Down Expand Up @@ -779,7 +779,7 @@ impl DiagCtxt {
// We delay a bug here so that `-Ztreat-err-as-bug -Zeagerly-emit-delayed-bugs`
// can be used to create a backtrace at the stashing site insted of whenever the
// diagnostic context is dropped and thus delayed bugs are emitted.
Error => Some(self.span_delayed_bug(span, "stashing {key:?}")),
Error => Some(self.span_delayed_bug(span, format!("stashing {key:?}"))),
DelayedBug => return self.inner.borrow_mut().emit_diagnostic(diag),
ForceWarning(_) | Warning | Note | OnceNote | Help | OnceHelp | FailureNote | Allow
| Expect(_) => None,
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::lang_items::LangItem;
use rustc_hir_analysis::check::{check_function_signature, forbid_intrinsic_abi};
use rustc_infer::infer::type_variable::TypeVariableOrigin;
use rustc_infer::infer::RegionVariableOrigin;
use rustc_infer::traits::WellFormedLoc;
use rustc_middle::ty::{self, Binder, Ty, TyCtxt};
use rustc_span::def_id::LocalDefId;
use rustc_span::symbol::sym;
Expand Down Expand Up @@ -71,6 +72,18 @@ pub(super) fn check_fn<'a, 'tcx>(
let inputs_hir = hir.fn_decl_by_hir_id(fn_id).map(|decl| &decl.inputs);
let inputs_fn = fn_sig.inputs().iter().copied();
for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {
// We checked the root's signature during wfcheck, but not the child.
if fcx.tcx.is_typeck_child(fn_def_id.to_def_id()) {
fcx.register_wf_obligation(
param_ty.into(),
param.span,
traits::WellFormed(Some(WellFormedLoc::Param {
function: fn_def_id,
param_idx: idx,
})),
);
}

// Check the pattern.
let ty: Option<&hir::Ty<'_>> = inputs_hir.and_then(|h| h.get(idx));
let ty_span = ty.map(|ty| ty.span);
Expand Down Expand Up @@ -108,7 +121,13 @@ pub(super) fn check_fn<'a, 'tcx>(
hir::FnRetTy::DefaultReturn(_) => body.value.span,
hir::FnRetTy::Return(ty) => ty.span,
};

fcx.require_type_is_sized(declared_ret_ty, return_or_body_span, traits::SizedReturnType);
// We checked the root's signature during wfcheck, but not the child.
if fcx.tcx.is_typeck_child(fn_def_id.to_def_id()) {
fcx.require_type_is_sized(declared_ret_ty, return_or_body_span, traits::WellFormed(None));
}

fcx.is_whole_body.set(true);
fcx.check_return_expr(body.value, false);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ pub enum WellFormedLoc {
/// The index of the parameter to use.
/// Parameters are indexed from 0, with the return type
/// being the last 'parameter'
param_idx: u16,
param_idx: usize,
},
}

Expand Down
5 changes: 0 additions & 5 deletions tests/crashes/123461.rs

This file was deleted.

20 changes: 20 additions & 0 deletions tests/ui/inference/issue-80409.compat.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0277]: the trait bound `usize: Fsm` is not satisfied
--> $DIR/issue-80409.rs:36:31
|
LL | builder.state().on_entry(|_| {});
| ^ the trait `Fsm` is not implemented for `usize`
|
help: this trait has no implementations, consider adding one
--> $DIR/issue-80409.rs:26:1
|
LL | trait Fsm {
| ^^^^^^^^^
note: required by a bound in `StateContext`
--> $DIR/issue-80409.rs:30:31
|
LL | struct StateContext<'a, TFsm: Fsm> {
| ^^^ required by this bound in `StateContext`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
24 changes: 15 additions & 9 deletions tests/ui/inference/issue-80409.no-compat.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
error: internal compiler error: error performing operation: fully_perform
--> $DIR/issue-80409.rs:49:30
error[E0277]: the trait bound `usize: Fsm` is not satisfied
--> $DIR/issue-80409.rs:36:31
|
LL | builder.state().on_entry(|_| {});
| ^^^
| ^ the trait `Fsm` is not implemented for `usize`
|
note:
--> $DIR/issue-80409.rs:49:30
help: this trait has no implementations, consider adding one
--> $DIR/issue-80409.rs:26:1
|
LL | builder.state().on_entry(|_| {});
| ^^^
LL | trait Fsm {
| ^^^^^^^^^
note: required by a bound in `StateContext`
--> $DIR/issue-80409.rs:30:31
|
LL | struct StateContext<'a, TFsm: Fsm> {
| ^^^ required by this bound in `StateContext`

error: aborting due to 1 previous error

query stack during panic:
end of query stack
For more information about this error, try `rustc --explain E0277`.
14 changes: 1 addition & 13 deletions tests/ui/inference/issue-80409.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
// This should not pass, because `usize: Fsm` does not hold. However, it currently ICEs.

// ignore-tidy-linelength

//@ revisions: compat no-compat
//@[compat] check-pass
//@[no-compat] compile-flags: -Zno-implied-bounds-compat
//@[no-compat] check-fail
//@[no-compat] known-bug: #80409
//@[no-compat] failure-status: 101
//@[no-compat] normalize-stderr-test "delayed at.*" -> ""
//@[no-compat] normalize-stderr-test "note: .*\n\n" -> ""
//@[no-compat] normalize-stderr-test "thread 'rustc' panicked.*\n" -> ""
//@[no-compat] normalize-stderr-test "(error: internal compiler error: [^:]+):\d+:\d+: " -> "$1:LL:CC: "
//@[no-compat] rustc-env:RUST_BACKTRACE=0

#![allow(unreachable_code, unused)]

Expand Down Expand Up @@ -47,4 +34,5 @@ struct StateContext<'a, TFsm: Fsm> {
fn main() {
let mut builder: FsmBuilder<usize> = todo!();
builder.state().on_entry(|_| {});
//~^ ERROR the trait bound `usize: Fsm` is not satisfied
}
1 change: 1 addition & 0 deletions tests/ui/issues/issue-66706.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ fn b() {
fn c() {
[0; [|&_: _ &_| {}; 0 ].len()]
//~^ ERROR expected `,`, found `&`
//~| ERROR type annotations needed
}

fn d() {
Expand Down
10 changes: 8 additions & 2 deletions tests/ui/issues/issue-66706.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | [0; [|&_: _ &_| {}; 0 ].len()]
| help: missing `,`

error: expected identifier, found reserved identifier `_`
--> $DIR/issue-66706.rs:18:26
--> $DIR/issue-66706.rs:19:26
|
LL | [0; match [|f @ &ref _| () ] {} ]
| ----- ^ expected identifier, found reserved identifier
Expand All @@ -34,6 +34,12 @@ error[E0282]: type annotations needed
LL | [0; [|_: _ &_| ()].len()]
| ^ cannot infer type

error: aborting due to 5 previous errors
error[E0282]: type annotations needed
--> $DIR/issue-66706.rs:13:11
|
LL | [0; [|&_: _ &_| {}; 0 ].len()]
| ^^^^^ cannot infer type

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0282`.
4 changes: 2 additions & 2 deletions tests/ui/traits/mutual-recursion-issue-75860.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ pub fn iso<A, B, F1, F2>(a: F1, b: F2) -> (Box<dyn Fn(A) -> B>, Box<dyn Fn(B) ->
(Box::new(a), Box::new(b))
}
pub fn iso_un_option<A, B>() -> (Box<dyn Fn(A) -> B>, Box<dyn Fn(B) -> A>) {
let left = |o_a: Option<_>| o_a.unwrap();
let left = |o_a: Option<_>| o_a.unwrap();
//~^ ERROR overflow
let right = |o_b: Option<_>| o_b.unwrap();
iso(left, right)
//~^ ERROR overflow
}

fn main() {}
12 changes: 4 additions & 8 deletions tests/ui/traits/mutual-recursion-issue-75860.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
error[E0275]: overflow evaluating the requirement `Option<_>: Sized`
--> $DIR/mutual-recursion-issue-75860.rs:11:5
error[E0275]: overflow assigning `_` to `Option<_>`
--> $DIR/mutual-recursion-issue-75860.rs:9:33
|
LL | iso(left, right)
| ^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`mutual_recursion_issue_75860`)
note: required by an implicit `Sized` bound in `Option`
--> $SRC_DIR/core/src/option.rs:LL:COL
LL | let left = |o_a: Option<_>| o_a.unwrap();
| ^^^

error: aborting due to 1 previous error

Expand Down
11 changes: 11 additions & 0 deletions tests/ui/transmute/ambiguity-in-closure-arg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Minimized test for <https://github.com/rust-lang/rust/issues/123461>.

struct Unconstrained<T>(T);

fn main() {
unsafe { std::mem::transmute::<_, ()>(|o_b: Unconstrained<_>| {}) };
//~^ ERROR type annotations needed
// We unfortunately don't check `Wf(Unconstrained<_>)`, so we won't
// hit an ambiguity error before checking the transmute. That means
// we still may have inference variables in our transmute src.
}
9 changes: 9 additions & 0 deletions tests/ui/transmute/ambiguity-in-closure-arg.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0282]: type annotations needed
--> $DIR/ambiguity-in-closure-arg.rs:6:44
|
LL | unsafe { std::mem::transmute::<_, ()>(|o_b: Unconstrained<_>| {}) };
| ^^^^^^^^^^^^^^^^^^^^^ cannot infer type

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0282`.
4 changes: 2 additions & 2 deletions tests/ui/type/type-check/unknown_type_for_closure.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0282]: type annotations needed
--> $DIR/unknown_type_for_closure.rs:2:13
--> $DIR/unknown_type_for_closure.rs:2:14
|
LL | let x = |b: Vec<_>| {};
| ^^^^^^^^^^^^^^ cannot infer type for struct `Vec<_>`
| ^^^^^^^^^ cannot infer type

error[E0282]: type annotations needed
--> $DIR/unknown_type_for_closure.rs:6:14
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/wf/closure-wf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
trait Bound {}
struct NeedsBound<T: Bound>(T);

// Checks that we enforce that closure args are WF.

fn constrain_inner<T, F: for<'a> FnOnce(&'a (), NeedsBound<T>)>(_: T, _: F) {}

fn main() {
constrain_inner(1u32, |&(), _| ());
//~^ ERROR the trait bound `u32: Bound` is not satisfied
}
20 changes: 20 additions & 0 deletions tests/ui/wf/closure-wf.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0277]: the trait bound `u32: Bound` is not satisfied
--> $DIR/closure-wf.rs:9:33
|
LL | constrain_inner(1u32, |&(), _| ());
| ^ the trait `Bound` is not implemented for `u32`
|
help: this trait has no implementations, consider adding one
--> $DIR/closure-wf.rs:1:1
|
LL | trait Bound {}
| ^^^^^^^^^^^
note: required by a bound in `NeedsBound`
--> $DIR/closure-wf.rs:2:22
|
LL | struct NeedsBound<T: Bound>(T);
| ^^^^^ required by this bound in `NeedsBound`

error: aborting due to 1 previous error

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

0 comments on commit 9e6c4fd

Please sign in to comment.