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

2022-03-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0891cd61b2f: [clang] [concepts] Check constrained-auto 
return types for void-returning… (authored by arthur.j.odwyer).

Changed prior to commit:
  https://reviews.llvm.org/D119184?vs=413006=413046#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,42 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C auto f2() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C auto f3() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C decltype(auto) f4() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C decltype(auto) f5() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C decltype(auto) f6() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C auto& f7() { // expected-error {{cannot form a reference to 'void'}}
+return void();
+  }
+  C auto& f8() {
+return; // expected-error {{cannot deduce return type 'C auto &' from omitted return expression}}
+  }
+  C auto& f9() { // expected-error {{cannot deduce return type 'C auto &' for function with no return statements}}
+  }
+}
+namespace PR53911 {
+  template concept C = false;
+
+  C auto *f1() {
+return (void*)nullptr; // FIXME: should error
+  }
+  C auto *f2() {
+return (int*)nullptr; // FIXME: should error
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3808,19 +3808,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
 if (!OrigResultType.getType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
 << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+  << OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14674,18 +14674,20 @@
   if (getLangOpts().CPlusPlus14) {
 if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
 FD->getReturnType()->isUndeducedType()) {
-  // If the function has a deduced result type but contains no 'return'
-  // statements, the result type as written must be exactly 'auto', and
-  // the deduced result type is 'void'.
+  // For a function with a deduced result type to return void,
+  // the result type as written must be 'auto' or 'decltype(auto)',
+  // possibly cv-qualified or constrained, but not ref-qualified.
   if (!FD->getReturnType()->getAs()) {
 Diag(dcl->getLocation(), diag::err_auto_fn_no_return_but_not_auto)
 << FD->getReturnType();
 FD->setInvalidDecl();
   } else {
-// Substitute 'void' for the 'auto' in the type.
-TypeLoc ResultType = getReturnTypeLoc(FD);
-Context.adjustDeducedFunctionResultType(
-FD, 

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

2022-03-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 413006.
Quuxplusone added a comment.
Herald added a project: All.

Rebased, poke CI one last time.
If this is green, imma land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,42 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C auto f2() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C auto f3() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C decltype(auto) f4() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C decltype(auto) f5() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C decltype(auto) f6() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C auto& f7() { // expected-error {{cannot form a reference to 'void'}}
+return void();
+  }
+  C auto& f8() {
+return; // expected-error {{cannot deduce return type 'C auto &' from omitted return expression}}
+  }
+  C auto& f9() { // expected-error {{cannot deduce return type 'C auto &' for function with no return statements}}
+  }
+}
+namespace PR53911 {
+  template concept C = false;
+
+  C auto *f1() {
+return (void*)nullptr; // FIXME: should error
+  }
+  C auto *f2() {
+return (int*)nullptr; // FIXME: should error
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3762,8 +3762,8 @@
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
 Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+const AutoType *AT) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3808,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
 if (!OrigResultType.getType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
 << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+  << OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14674,18 +14674,20 @@
   if (getLangOpts().CPlusPlus14) {
 if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
 FD->getReturnType()->isUndeducedType()) {
-  // If the function has a deduced result type but contains no 'return'
-  // statements, the result type as written must be exactly 'auto', and
-  // the deduced result type is 'void'.
+  // For a function with a deduced result type to return void,
+  

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

2022-02-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 409740.
Quuxplusone added a comment.

Rebase and update — this is becoming more and more of a trivial patch, which I 
guess is good!
Add a test case for https://github.com/llvm/llvm-project/issues/53911 (which I 
finally thought to test, and was surprised to find it didn't work either before 
//or// after my patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,42 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C auto f2() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C auto f3() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C decltype(auto) f4() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return void();
+  }
+  C decltype(auto) f5() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+return;
+  }
+  C decltype(auto) f6() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  }
+  C auto& f7() { // expected-error {{cannot form a reference to 'void'}}
+return void();
+  }
+  C auto& f8() {
+return; // expected-error {{cannot deduce return type 'C auto &' from omitted return expression}}
+  }
+  C auto& f9() { // expected-error {{cannot deduce return type 'C auto &' for function with no return statements}}
+  }
+}
+namespace PR53911 {
+  template concept C = false;
+
+  C auto *f1() {
+return (void*)nullptr; // FIXME: should error
+  }
+  C auto *f2() {
+return (int*)nullptr; // FIXME: should error
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3762,8 +3762,8 @@
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
 Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+const AutoType *AT) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3808,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
 if (!OrigResultType.getType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
 << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+  << OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14660,18 +14660,20 @@
   if (getLangOpts().CPlusPlus14) {
 if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
 FD->getReturnType()->isUndeducedType()) {
-  // If the function has a deduced result type but contains no 'return'
-  // 

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

2022-02-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116
 
-auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' 
for function with no return statements}}
+auto _ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 
'void'

sammccall wrote:
> Quuxplusone wrote:
> > sammccall wrote:
> > > 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)
> > I agree about the (mild) regression but am not sure what to do about it. I 
> > think it might help if Clang emitted a note `note: during return type 
> > deduction here` (pointer to the offending `return` statement or 
> > end-of-function); do you think I should do that? and if so, how?
> > 
> > > It may be worth keeping the explicit check in `ActOnFinishFunctionBody` 
> > > before the deduction happens to improve the diagnostic in this case.
> > 
> > I can't picture what you mean; can you suggest a specific patch?
> > 
> > In case it matters, I'm a little leery of hard-coding a special case like 
> > we had in the old `DeduceFunctionTypeFromReturnExpr` lines 3814–3816,
> > ```
> > // Deduction here can only succeed if the return type is exactly 'cv 
> > auto'
> > // or 'decltype(auto)', so just check for that case directly.
> > if (!OrigResultType.getType()->getAs()) {
> > ```
> > because special-casing `void` is kind of how we got into this mess in the 
> > first place.
> > I can't picture what you mean; can you suggest a specific patch?
> > In case it matters, I'm a little leery of hard-coding a special case like 
> > we had in the old
> > DeduceFunctionTypeFromReturnExpr
> 
> Yeah, so that was pretty much my suggestion: keep the `getAs()` 
> check in `ActOnFinishFunctionBody` to reject the most common cases with a 
> good diagnostic, and then if it passes do the actual deduction.
> I agree this is ugly, I just didn't really have a better idea.
> 
> > I think it might help if Clang emitted a note note: during return type 
> > deduction here 
> 
> This is a nice idea, maybe better than the existing diagnostic.
> 
> I haven't added notes myself, but I think all you have to do is `add a 
> note_*` entry to DiagnosticSemaKinds.td, and emit it after the primary 
> diagnostic as you'd emit any other diagnostic.
> Here if you see `DAR_Failed` then I think you know the call emitted a 
> diagnostic and you can add the note. I'm not sure about 
> `DAR_FailedAlreadyDiagnosed` - if it means no new diagnostic was emitted then 
> you also don't want to add your note.
> 
> (You'd want to do this in the `if (RetExpr)` case too of course)
> 
> > pointer to the offending return statement or end-of-function
> 
> The main diagnostics is going to point at that location already. So it might 
> be more helpful to point the "during return type deduction" note at the 
> return type written in the function declaration:
> 
> ```
>   return;
>   ^~
> error: cannot form reference to 'void'
> 
> auto& foo() {
>   ^~~~
> note: deducing return type for 'foo'
> ```
@rsmith: This is the discussion that led to D119778 adding the note. I suppose 
that by keeping all the special-case error messages in 
`ActOnFinishFunctionBody`, we've reduced the benefit of the note.
In particular, `void_ret_2` no longer has this "regression," so there's no 
longer any need for the note here — in fact, as you mentioned, the note is a 
bit noisy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3766
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its

rsmith wrote:
> It looks like you ended up not using the new parameter for anything. Did you 
> intend to / do you still need it?
Huh, I guess I don't need it anymore.
Btw, does it make sense that I marked `AT` as `const AutoType *`? Initially I'd 
been worried that something on this codepath might be stashing state inside 
`*AT`, or otherwise updating it to say "This was //written// `auto&`, but now 
//refers// to the type ." This week, I'm not sure why I would have ever 
suspected that.
But still, this codepath //does// update the state of `*FD`, so the fact that 
it doesn't update `*AT` is still somewhat worthy of note...
Anyway, if the `const` is uncontroversial, I could just land that `const` in 
its own commit to shrink this PR a tiny bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3766
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its

It looks like you ended up not using the new parameter for anything. Did you 
intend to / do you still need it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 409090.
Quuxplusone added a comment.

Rebase; fix the clang-format nit; reinsert a missing `FD->setInvalidDecl()` 
that seemed to be causing crashes in Modules code under libcxx/test/ but 
otherwise doesn't seem to be tested in clang/test/ :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,23 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+
+  void g() {
+f1();
+f2();
+f3();
+f4();
+f5();
+f6();
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,8 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT,
+ /*HasReturnStmt=*/true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3761,9 +3762,9 @@
 /// C++1y [dcl.spec.auto]p6.
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
-Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3809,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
+if (!FD->getReturnType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
+  << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+  << OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
@@ -3989,7 +3997,8 @@
   // trying to deduce its return type.
   // (Some return values may be needlessly wrapped in RecoveryExpr).
   if (FD->isInvalidDecl() ||
-  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+  

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

2022-02-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 408594.
Quuxplusone marked an inline comment as done and an inline comment as not done.
Quuxplusone added a comment.

Address review comments; add exhaustive tests as a parent revision.


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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,23 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+
+  void g() {
+f1();
+f2();
+f3();
+f4();
+f5();
+f6();
+  }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,8 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT,
+ /*HasReturnStmt=*/true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3761,9 +3762,9 @@
 /// C++1y [dcl.spec.auto]p6.
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
-Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3809,26 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
+// For a function with a deduced result type to return void,
+// the result type as written must be 'auto' or 'decltype(auto)',
+// possibly cv-qualified or constrained, but not ref-qualified.
+if (!FD->getReturnType()->getAs()) {
   Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
+  << OrigResultType.getType();
   return true;
 }
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+Expr *Dummy = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+
+if (DAR == DAR_Failed && !FD->isInvalidDecl())
+  Diag(ReturnLoc, diag::err_auto_fn_deduction_failure)
+<< OrigResultType.getType() << Dummy->getType();
+
+if (DAR != DAR_Succeeded)
   return true;
   }
 
@@ -3989,7 +3997,8 @@
   // trying to deduce its return type.
   // (Some return values may be needlessly wrapped in RecoveryExpr).
   if (FD->isInvalidDecl() ||
-  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT,
+   /*HasReturnStmt=*/true)) 

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

2022-02-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3816
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), 
ReturnLoc);
+Expr *Dummy = R.get();
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);

Quuxplusone wrote:
> sammccall wrote:
> > why initialize into an ExprResult instead of an Expr* directly?
> Because I didn't know the latter was possible. :) Btw, I assume there's no 
> worry here about how the Expr will ultimately get garbage-collected, right? 
> All the code I looked at seems pretty cavalier about it, so I assume `new 
> (Context)` magically takes care of the memory management part.
Allocating a node here as if the user wrote `return void();` seems reasonable 
to me, and you don't have to worry about cleaning it up.

AST nodes are allocated on an arena owned by the ASTContext and never 
individually destroyed, the arena (BumpPtrAllocator) is just destroyed at the 
end along with the ASTContext.

(In fact the IIRC the default `clang -cc1` behavior is not even to destroy 
ASTContext, Sema etc and just exit, see `FrontendOpts::DisableFree`)



Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116
 
-auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' 
for function with no return statements}}
+auto _ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 
'void'

Quuxplusone wrote:
> sammccall wrote:
> > 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)
> I agree about the (mild) regression but am not sure what to do about it. I 
> think it might help if Clang emitted a note `note: during return type 
> deduction here` (pointer to the offending `return` statement or 
> end-of-function); do you think I should do that? and if so, how?
> 
> > It may be worth keeping the explicit check in `ActOnFinishFunctionBody` 
> > before the deduction happens to improve the diagnostic in this case.
> 
> I can't picture what you mean; can you suggest a specific patch?
> 
> In case it matters, I'm a little leery of hard-coding a special case like we 
> had in the old `DeduceFunctionTypeFromReturnExpr` lines 3814–3816,
> ```
> // Deduction here can only succeed if the return type is exactly 'cv auto'
> // or 'decltype(auto)', so just check for that case directly.
> if (!OrigResultType.getType()->getAs()) {
> ```
> because special-casing `void` is kind of how we got into this mess in the 
> first place.
> I can't picture what you mean; can you suggest a specific patch?
> In case it matters, I'm a little leery of hard-coding a special case like we 
> had in the old
> DeduceFunctionTypeFromReturnExpr

Yeah, so that was pretty much my suggestion: keep the `getAs()` check 
in `ActOnFinishFunctionBody` to reject the most common cases with a good 
diagnostic, and then if it passes do the actual deduction.
I agree this is ugly, I just didn't really have a better idea.

> I think it might help if Clang emitted a note note: during return type 
> deduction here 

This is a nice idea, maybe better than the existing diagnostic.

I haven't added notes myself, but I think all you have to do is `add a note_*` 
entry to DiagnosticSemaKinds.td, and emit it after the primary diagnostic as 
you'd emit any other diagnostic.
Here if you see `DAR_Failed` then I think you know the call emitted a 
diagnostic and you can add the note. I'm not sure about 
`DAR_FailedAlreadyDiagnosed` - if it means no new diagnostic was emitted then 
you also don't want to add your note.

(You'd want to do this in the `if (RetExpr)` case too of course)

> pointer to the offending return statement or end-of-function

The main diagnostics is going to point at that location already. So it might be 
more helpful to point the "during return type deduction" note at the return 
type written in the function declaration:

```
  return;
  ^~
error: cannot form reference to 'void'

auto& foo() {
  ^~~~
note: deducing return type for 'foo'
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone marked an inline comment as not done.
Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3816
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), 
ReturnLoc);
+Expr *Dummy = R.get();
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);

sammccall wrote:
> why initialize into an ExprResult instead of an Expr* directly?
Because I didn't know the latter was possible. :) Btw, I assume there's no 
worry here about how the Expr will ultimately get garbage-collected, right? All 
the code I looked at seems pretty cavalier about it, so I assume `new 
(Context)` magically takes care of the memory management part.



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)

sammccall wrote:
> (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)
Will do.



Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116
 
-auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' 
for function with no return statements}}
+auto _ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 
'void'

sammccall wrote:
> 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)
I agree about the (mild) regression but am not sure what to do about it. I 
think it might help if Clang emitted a note `note: during return type deduction 
here` (pointer to the offending `return` statement or end-of-function); do you 
think I should do that? and if so, how?

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

I can't picture what you mean; can you suggest a specific patch?

In case it matters, I'm a little leery of hard-coding a special case like we 
had in the old `DeduceFunctionTypeFromReturnExpr` lines 3814–3816,
```
// Deduction here can only succeed if the return type is exactly 'cv auto'
// or 'decltype(auto)', so just check for that case directly.
if (!OrigResultType.getType()->getAs()) {
```
because special-casing `void` is kind of how we got into this mess in the first 
place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-10 Thread 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 _ret_2() {} // expected-error {{cannot deduce return type 'auto &' 
for function with no return statements}}
+auto _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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-02-09 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 407284.
Quuxplusone added a comment.

Rebase; clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,23 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+
+  C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+
+  void g() {
+f1();
+f2();
+f3();
+f4();
+f5();
+f6();
+  }
+}
Index: clang/test/SemaCXX/deduced-return-type-cxx14.cpp
===
--- clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -113,7 +113,7 @@
 using Void = void;
 using Void = decltype(void_ret());
 
-auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' for function with no return statements}}
+auto _ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 'void'
 
 const auto void_ret_4() {
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,7 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT, true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3761,9 +3761,9 @@
 /// C++1y [dcl.spec.auto]p6.
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
-Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+Expr *, const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3808,18 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
-  Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
-  return true;
-}
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+ExprResult R = new (Context) CXXScalarValueInitExpr(
+Context.VoidTy,
+Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc);
+Expr *Dummy = R.get();
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+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)
+  << OrigResultType.getType();
+if (DAR != DAR_Succeeded)
   return true;
   }
 
@@ -3988,8 +3987,8 @@
   // we saw a `return` whose 

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

2022-02-08 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 406847.
Quuxplusone added a comment.

Update one additional test where the expected output has changed:

  auto& f() { }

now produces "can't make a reference to void" instead of "can't deduce `auto&` 
from no return statements." I think this is a mild regression in diagnostic 
quality, but I don't immediately see what we should do about it so I'm inclined 
to take the hit. Suggestions welcome.


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

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,14 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+  C auto f1() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() { } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() { } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  void g() { f1(); f2(); f3(); f4(); f5(); f6(); }
+}
Index: clang/test/SemaCXX/deduced-return-type-cxx14.cpp
===
--- clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -113,7 +113,7 @@
 using Void = void;
 using Void = decltype(void_ret());
 
-auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' for function with no return statements}}
+auto _ret_2() {} // expected-error {{cannot form a reference to 'void'}}
 const auto void_ret_3() {} // ok, return type 'const void' is adjusted to 'void'
 
 const auto void_ret_4() {
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,7 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT, true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3762,8 +3762,9 @@
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
 Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3809,18 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
-  Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
-  return true;
-}
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+ExprResult R = new (Context) CXXScalarValueInitExpr(
+  Context.VoidTy, Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc),
+  ReturnLoc);
+Expr *Dummy = R.get();
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+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)
+  << 

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

2022-02-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone created this revision.
Quuxplusone added reviewers: sammccall, saar.raz, rsmith, mizvekov, majnemer, 
riccibruno.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/49188


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119184

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -169,3 +169,14 @@
   template void f(T, U) = delete;
   void g() { f(0, 0); }
 }
+
+namespace PR49188 {
+  template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
+  C auto f1() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f2() { return; } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C auto f3() { } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f5() { return; } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  C decltype(auto) f6() { } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  void g() { f1(); f2(); f3(); f4(); f5(); f6(); }
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3590,7 +3590,7 @@
 
 AutoType *AT = CurCap->ReturnType->getContainedAutoType();
 assert(AT && "lost auto type from lambda return type");
-if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT, true)) {
   FD->setInvalidDecl();
   // FIXME: preserve the ill-formed return expression.
   return StmtError();
@@ -3762,8 +3762,9 @@
 bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
 SourceLocation ReturnLoc,
 Expr *,
-AutoType *AT) {
-  // If this is the conversion function for a lambda, we choose to deduce it
+const AutoType *AT,
+bool HasReturnStmt) {
+  // If this is the conversion function for a lambda, we choose to deduce its
   // type from the corresponding call operator, not from the synthesized return
   // statement within it. See Sema::DeduceReturnType.
   if (isLambdaConversionOperator(FD))
@@ -3808,19 +3809,18 @@
 LocalTypedefNameReferencer Referencer(*this);
 Referencer.TraverseType(RetExpr->getType());
   } else {
-//  In the case of a return with no operand, the initializer is considered
-//  to be void().
-//
-// Deduction here can only succeed if the return type is exactly 'cv auto'
-// or 'decltype(auto)', so just check for that case directly.
-if (!OrigResultType.getType()->getAs()) {
-  Diag(ReturnLoc, diag::err_auto_fn_return_void_but_not_auto)
-<< OrigResultType.getType();
-  return true;
-}
-// We always deduce U = void in this case.
-Deduced = SubstAutoType(OrigResultType.getType(), Context.VoidTy);
-if (Deduced.isNull())
+// In the case of a return with no operand, the initializer is considered
+// to be 'void()'.
+ExprResult R = new (Context) CXXScalarValueInitExpr(
+  Context.VoidTy, Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc),
+  ReturnLoc);
+Expr *Dummy = R.get();
+DeduceAutoResult DAR = DeduceAutoType(OrigResultType, Dummy, Deduced);
+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)
+  << OrigResultType.getType();
+if (DAR != DAR_Succeeded)
   return true;
   }
 
@@ -3989,7 +3989,7 @@
   // trying to deduce its return type.
   // (Some return values may be needlessly wrapped in RecoveryExpr).
   if (FD->isInvalidDecl() ||
-  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
+  DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT, true)) {
 FD->setInvalidDecl();
 if (!AllowRecovery)
   return StmtError();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14660,18 +14660,12 @@
   if (getLangOpts().CPlusPlus14) {
 if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&