[PATCH] D119184: [clang] [concepts] Check constrained-auto return types for void-returning functions

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

This looks right to me, though I think we should spend a few more lines to preserve the nice diagnostics for simple cases.



================
Comment at: clang/lib/Sema/SemaStmt.cpp:3593
     assert(AT && "lost auto type from lambda return type");
-    if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+    if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT, true)) {
       FD->setInvalidDecl();
----------------
nit: /*HasReturnStatement=*/true?


================
Comment at: clang/lib/Sema/SemaStmt.cpp:3816
+        Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+    Expr *Dummy = R.get();
+    DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
----------------
why initialize into an ExprResult instead of an Expr* directly?


================
Comment at: clang/lib/Sema/SemaStmt.cpp:3819
+    if (DAR == DAR_Failed && !FD->isInvalidDecl())
+      Diag(ReturnLoc, HasReturnStmt ? diag::err_auto_fn_return_void_but_not_auto
+                                    : diag::err_auto_fn_no_return_but_not_auto)
----------------
(BTW, err_auto_fn_return_void_but_not_auto has no testcases, feel free to add one near `deduced-return-type-cxx14.cpp:341`, or not)


================
Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116
 
-auto &void_ret_2() {} // expected-error {{cannot deduce return type 'auto &' for function with no return statements}}
+auto &void_ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 'void'
----------------
This feels like a regression, this diagnostic is just attached to the end of the function and there's no longer any explicit indication that the return type or deduction is involved.

It may be worth keeping the explicit check in `ActOnFinishFunctionBody` before the deduction happens to improve the diagnostic in this case.

(I don't think it's important to do this in the return-with-no-argument case, since the error will point at the return statement which provides enough context I think)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119184/new/

https://reviews.llvm.org/D119184