Skip to content

Commit

Permalink
Rollup merge of #103445 - fmease:fix-50291, r=estebank
Browse files Browse the repository at this point in the history
`#[test]`: Point at return type if `Termination` bound is unsatisfied

Together with #103142 (already merged) this fully fixes #50291.

I don't consider my current solution of changing a few spans “here and there” very clean since the
failed obligation is a `FunctionArgumentObligation` and we point at a type instead of a function argument.

If you agree with me on this point, I can offer to keep the spans of the existing nodes and instead inject
`let _: AssertRetTyIsTermination<$ret_ty>;` (type to be defined in `libtest`) similar to `AssertParamIsEq` etc.
used by some built-in derive-macros.

I haven't tried that approach yet though and cannot promise that it would actually work out or
be “cleaner” for that matter.

````@rustbot```` label A-libtest A-diagnostics
r? ````@estebank````
  • Loading branch information
Dylan-DPC authored Nov 11, 2022
2 parents 978e7ab + 449a440 commit bc9567f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 13 deletions.
18 changes: 12 additions & 6 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub fn expand_test_or_bench(
};

// Note: non-associated fn items are already handled by `expand_test_or_bench`
if !matches!(item.kind, ast::ItemKind::Fn(_)) {
let ast::ItemKind::Fn(fn_) = &item.kind else {
let diag = &cx.sess.parse_sess.span_diagnostic;
let msg = "the `#[test]` attribute may only be used on a non-associated function";
let mut err = match item.kind {
Expand All @@ -130,7 +130,7 @@ pub fn expand_test_or_bench(
.emit();

return vec![Annotatable::Item(item)];
}
};

// has_*_signature will report any errors in the type so compilation
// will fail. We shouldn't try to expand in this case because the errors
Expand All @@ -141,12 +141,14 @@ pub fn expand_test_or_bench(
return vec![Annotatable::Item(item)];
}

let (sp, attr_sp) = (cx.with_def_site_ctxt(item.span), cx.with_def_site_ctxt(attr_sp));
let sp = cx.with_def_site_ctxt(item.span);
let ret_ty_sp = cx.with_def_site_ctxt(fn_.sig.decl.output.span());
let attr_sp = cx.with_def_site_ctxt(attr_sp);

let test_id = Ident::new(sym::test, attr_sp);

// creates test::$name
let test_path = |name| cx.path(sp, vec![test_id, Ident::from_str_and_span(name, sp)]);
let test_path = |name| cx.path(ret_ty_sp, vec![test_id, Ident::from_str_and_span(name, sp)]);

// creates test::ShouldPanic::$name
let should_panic_path = |name| {
Expand Down Expand Up @@ -192,7 +194,7 @@ pub fn expand_test_or_bench(
vec![
// super::$test_fn(b)
cx.expr_call(
sp,
ret_ty_sp,
cx.expr_path(cx.path(sp, vec![item.ident])),
vec![cx.expr_ident(sp, b)],
),
Expand All @@ -216,7 +218,11 @@ pub fn expand_test_or_bench(
cx.expr_path(test_path("assert_test_result")),
vec![
// $test_fn()
cx.expr_call(sp, cx.expr_path(cx.path(sp, vec![item.ident])), vec![]), // )
cx.expr_call(
ret_ty_sp,
cx.expr_path(cx.path(sp, vec![item.ident])),
vec![],
), // )
],
), // }
), // )
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
error[E0277]: the trait bound `f32: Termination` is not satisfied
--> $DIR/termination-trait-test-wrong-type.rs:6:1
--> $DIR/termination-trait-test-wrong-type.rs:6:31
|
LL | #[test]
| ------- in this procedural macro expansion
LL | / fn can_parse_zero_as_f32() -> Result<f32, ParseFloatError> {
LL | | "0".parse()
LL | | }
| |_^ the trait `Termination` is not implemented for `f32`
LL | #[test]
| ------- in this procedural macro expansion
LL | fn can_parse_zero_as_f32() -> Result<f32, ParseFloatError> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Termination` is not implemented for `f32`
|
= note: required for `Result<f32, ParseFloatError>` to implement `Termination`
note: required by a bound in `assert_test_result`
Expand Down

0 comments on commit bc9567f

Please sign in to comment.