[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

@theraven: Can you post a minimal repro of your case? I don't follow your 
distinction between "caller" and "enclosing function."

Regarding `noreturn` and `always_inline`: maybe the rules for `musttail` could 
be relaxed in cases like the one you mention, but it would require changing the 
backend (LLVM). Here I changed the front-end only and used LLVM's existing 
`musttail` support, which meant accepting its existing limitations 
.

I would love to see an exception for `always_inline`: my use case would benefit 
greatly from this. In my own project I had to change a bunch of `always_inline` 
functions to macros to work around this rule. Unfortunately this is complicated 
by the fact that `always_inline` does not actually guarantee that inlining 
occurs 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

> Such a definition doesn't make much sense because for some targets tail call 
> optimization isn't available at all.
> Eg. Wasm w/o tail call proposal, xtensa windowed abi.

That is true. But what is the correct behavior if you try to use 
`[[clang::musttail]]` on such a target? In my case, I want to get a compiler 
error, because if a tail call cannot be performed, the algorithm is incorrect 
(it will cause stack overflow at runtime).

There are three possible semantics that I can see:

1. Perform a tail call if possible.
2. Perform a tail call, fail to compile if this is not possible 
(`[[clang::nonportable_musttail]]`)
3. Perform a tail call, fail to compile if this is not possible //or// if the 
function signatures between caller and callee differ in ways that make tail 
calls difficult on some platforms (`[[clang::musttail]]`)

Both (2) and (3) would fail to compile on Wasm without tail calls. If all you 
want is (1), that doesn't require any attribute (the optimizer will do it 
automatically).


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

https://reviews.llvm.org/D147714

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

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

I think I misunderstood @yamt's comment in 
https://reviews.llvm.org/D147714#4446780. I take back what I wrote above.

I agree that `[[clang::nonportable_musttail]]` is a nicer semantic, and the 
restrictions around the existing `[[clang:musttail]]` don't seem to buy us very 
much, since they are not universal enough to give a true assurance.

If someone could land a change to both LLVM and Clang to change the existing 
attribute, I would have no objection. But I have no idea what is involved in 
landing a backend (LLVM) change of that magnitude, especially since it would 
touch all the arch-specific backends.


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

https://reviews.llvm.org/D147714

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

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

Correct me if I am wrong, but won't this produce LLVM IR that fails to validate?

The checks that exist today for `[[clang::musttail]]` are necessary to follow 
LLVM's rules about where `musttail` may appear: 
https://llvm.org/docs/LangRef.html#id328

> Calls marked musttail must obey the following additional rules:
>
> - The call must immediately precede a ret instruction, or a pointer bitcast 
> followed by a ret instruction.
> - The ret instruction must return the (possibly bitcasted) value produced by 
> the call, undef, or void.
> - The calling conventions of the caller and callee must match.
> - The callee must be varargs iff the caller is varargs. Bitcasting a 
> non-varargs function to the appropriate varargs type is legal so long as the 
> non-varargs prefixes obey the other rules.
> - The return type must not undergo automatic conversion to an sret pointer.
>
> In addition, if the calling convention is not swifttailcc or tailcc:
>
> - All ABI-impacting function attributes, such as sret, byval, inreg, 
> returned, and inalloca, must match.
> - The caller and callee prototypes must match. Pointer types of parameters or 
> return types may differ in pointee type, but not in address space.

It looks like this change would allow users to produce LLVM IR that violates 
that final condition:

> The caller and callee prototypes must match. Pointer types of parameters or 
> return types may differ in pointee type, but not in address space.


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

https://reviews.llvm.org/D147714

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

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

> is a [[should_tail]] attribute sort of thing: a tail-hint where we do 'best 
> effort with no promises', and make no guarantees that we're going to tail it.

I'm not sure I see the value in that. The compiler already optimizes tail calls 
when it can in a best-effort manner. The purpose of `[[musttail]]` is to 
support algorithms that would blow the stack if tail calls were not optimized. 
It's better to get a compiler error than to get a stack overflow at runtime 
(especially if stack overflow only occurs with certain inputs).


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

https://reviews.llvm.org/D147714

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

[PATCH] D147714: [Attr] Introduce [[clang::nonportable_musttail]] as less strict version of [[clang::musttail]]

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

`[[nonportable_musttail]]` makes sense to me as a semantic. It indicates that 
the algorithm requires tail calls, but the author is willing to accept that the 
algorithm may be non-portable.

"Non-portable" here can mean architecture-specific, but it can also mean 
"sensitive to compiler flags." For example, I think there are architectures 
where tail calls can be optimized for statically-linked code but not across a 
shared library boundary.

Since the circumstances for when tail calls are supported can be complicated 
and subtle, architecture-specific attributes don't make as much sense to me.


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

https://reviews.llvm.org/D147714

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

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

[Josh Haberman via Phabricator via cfe-commits]

haberman created this revision.
haberman added a reviewer: rsmith.
haberman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is no functional change here (hence no new tests). The only change
is to replace a couple uintptr_t members with llvm::PointerIntPair<> to
clean up the code, making it more readable and less error prone.

This cleanup highlighted that the old code was effectively casting away
const. This is fixed by changing some function signatures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98889

Files:
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaInit.cpp

Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -24,6 +24,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
@@ -3281,10 +3282,7 @@
   InitializedEntity Result;
   Result.Kind = EK_Base;
   Result.Parent = Parent;
-  Result.Base = reinterpret_cast(Base);
-  if (IsInheritedVirtualBase)
-Result.Base |= 0x01;
-
+  Result.Base = {Base, IsInheritedVirtualBase};
   Result.Type = Base->getType();
   return Result;
 }
@@ -3293,7 +3291,7 @@
   switch (getKind()) {
   case EK_Parameter:
   case EK_Parameter_CF_Audited: {
-ParmVarDecl *D = reinterpret_cast(Parameter & ~0x1);
+ParmVarDecl *D = Parameter.getPointer();
 return (D ? D->getDeclName() : DeclarationName());
   }
 
@@ -3336,7 +3334,7 @@
 
   case EK_Parameter:
   case EK_Parameter_CF_Audited:
-return reinterpret_cast(Parameter & ~0x1);
+return Parameter.getPointer();
 
   case EK_Result:
   case EK_StmtExprResult:
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -254,8 +254,7 @@
 ComputedEST = EST_None;
 }
 
-ExprResult Sema::ConvertParamDefaultArgument(const ParmVarDecl *Param,
- Expr *Arg,
+ExprResult Sema::ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
  SourceLocation EqualLoc) {
   if (RequireCompleteType(Param->getLocation(), Param->getType(),
   diag::err_typecheck_decl_incomplete_type))
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2702,8 +2702,7 @@
   void ActOnParamUnparsedDefaultArgument(Decl *param, SourceLocation EqualLoc,
  SourceLocation ArgLoc);
   void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
-  ExprResult ConvertParamDefaultArgument(const ParmVarDecl *Param,
- Expr *DefaultArg,
+  ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
  SourceLocation EqualLoc);
   void SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
SourceLocation EqualLoc);
Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -188,7 +188,7 @@
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
 /// low bit indicating whether the parameter is "consumed".
-uintptr_t Parameter;
+llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
 /// source information for the temporary.
@@ -199,7 +199,7 @@
 /// When Kind == EK_Base, the base specifier that provides the
 /// base class. The lower bit specifies whether the base is an inherited
 /// virtual base.
-uintptr_t Base;
+llvm::PointerIntPair Base;
 
 /// When Kind == EK_ArrayElement, EK_VectorElement, or
 /// EK_ComplexElement, the index of the array or vector element being
@@ -252,15 +252,14 @@
 
   /// Create the initialization entity for a parameter.
   static InitializedEntity InitializeParameter(ASTContext &Context,
-   const ParmVarDecl *Parm) {
+   ParmVarDecl *Parm) {
 return InitializeParameter(Context, Parm, Parm->getType());
   }
 
   /// Create the initialization entity for a parameter, but use
   /// another type.
-  static InitializedEntity InitializeParameter(ASTContext &Context,
-   const ParmVarDecl *Parm,
-  

[PATCH] D98893: Updated comment "the low bit" -> "the integer".

[Josh Haberman via Phabricator via cfe-commits]

haberman created this revision.
haberman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98893

Files:
  clang/include/clang/Sema/Initialization.h


Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -187,7 +187,7 @@
 ObjCMethodDecl *MethodDecl;
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
-/// low bit indicating whether the parameter is "consumed".
+/// integer indicating whether the parameter is "consumed".
 llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
@@ -197,7 +197,7 @@
 struct LN LocAndNRVO;
 
 /// When Kind == EK_Base, the base specifier that provides the
-/// base class. The lower bit specifies whether the base is an inherited
+/// base class. The integer specifies whether the base is an inherited
 /// virtual base.
 llvm::PointerIntPair Base;
 


Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -187,7 +187,7 @@
 ObjCMethodDecl *MethodDecl;
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
-/// low bit indicating whether the parameter is "consumed".
+/// integer indicating whether the parameter is "consumed".
 llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
@@ -197,7 +197,7 @@
 struct LN LocAndNRVO;
 
 /// When Kind == EK_Base, the base specifier that provides the
-/// base class. The lower bit specifies whether the base is an inherited
+/// base class. The integer specifies whether the base is an inherited
 /// virtual base.
 llvm::PointerIntPair Base;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98893: Updated comment "the low bit" -> "the integer".

[Josh Haberman via Phabricator via cfe-commits]

haberman abandoned this revision.
haberman added a comment.

This was meant to be an update on https://reviews.llvm.org/D98889


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98893

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

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 331679.
haberman added a comment.

Updated comment "the low bit" -> "the integer".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98889

Files:
  clang/include/clang/Sema/Initialization.h


Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -187,7 +187,7 @@
 ObjCMethodDecl *MethodDecl;
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
-/// low bit indicating whether the parameter is "consumed".
+/// integer indicating whether the parameter is "consumed".
 llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
@@ -197,7 +197,7 @@
 struct LN LocAndNRVO;
 
 /// When Kind == EK_Base, the base specifier that provides the
-/// base class. The lower bit specifies whether the base is an inherited
+/// base class. The integer specifies whether the base is an inherited
 /// virtual base.
 llvm::PointerIntPair Base;
 


Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -187,7 +187,7 @@
 ObjCMethodDecl *MethodDecl;
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
-/// low bit indicating whether the parameter is "consumed".
+/// integer indicating whether the parameter is "consumed".
 llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
@@ -197,7 +197,7 @@
 struct LN LocAndNRVO;
 
 /// When Kind == EK_Base, the base specifier that provides the
-/// base class. The lower bit specifies whether the base is an inherited
+/// base class. The integer specifies whether the base is an inherited
 /// virtual base.
 llvm::PointerIntPair Base;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

[Josh Haberman via Phabricator via cfe-commits]

haberman marked an inline comment as done.
haberman added a comment.

> Thanks! Do we need InitializedEntity::getDecl to return a pointer to 
> non-const?

Yes, if I try to propagate `const` the change starts to snowball.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98889

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

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 332405.
haberman added a comment.

Updated change to reflect all diffs since main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98889

Files:
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaInit.cpp

Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -24,6 +24,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
@@ -3281,10 +3282,7 @@
   InitializedEntity Result;
   Result.Kind = EK_Base;
   Result.Parent = Parent;
-  Result.Base = reinterpret_cast(Base);
-  if (IsInheritedVirtualBase)
-Result.Base |= 0x01;
-
+  Result.Base = {Base, IsInheritedVirtualBase};
   Result.Type = Base->getType();
   return Result;
 }
@@ -3293,7 +3291,7 @@
   switch (getKind()) {
   case EK_Parameter:
   case EK_Parameter_CF_Audited: {
-ParmVarDecl *D = reinterpret_cast(Parameter & ~0x1);
+ParmVarDecl *D = Parameter.getPointer();
 return (D ? D->getDeclName() : DeclarationName());
   }
 
@@ -3336,7 +3334,7 @@
 
   case EK_Parameter:
   case EK_Parameter_CF_Audited:
-return reinterpret_cast(Parameter & ~0x1);
+return Parameter.getPointer();
 
   case EK_Result:
   case EK_StmtExprResult:
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -254,8 +254,7 @@
 ComputedEST = EST_None;
 }
 
-ExprResult Sema::ConvertParamDefaultArgument(const ParmVarDecl *Param,
- Expr *Arg,
+ExprResult Sema::ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
  SourceLocation EqualLoc) {
   if (RequireCompleteType(Param->getLocation(), Param->getType(),
   diag::err_typecheck_decl_incomplete_type))
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2702,8 +2702,7 @@
   void ActOnParamUnparsedDefaultArgument(Decl *param, SourceLocation EqualLoc,
  SourceLocation ArgLoc);
   void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
-  ExprResult ConvertParamDefaultArgument(const ParmVarDecl *Param,
- Expr *DefaultArg,
+  ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
  SourceLocation EqualLoc);
   void SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
SourceLocation EqualLoc);
Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -187,8 +187,8 @@
 ObjCMethodDecl *MethodDecl;
 
 /// When Kind == EK_Parameter, the ParmVarDecl, with the
-/// low bit indicating whether the parameter is "consumed".
-uintptr_t Parameter;
+/// integer indicating whether the parameter is "consumed".
+llvm::PointerIntPair Parameter;
 
 /// When Kind == EK_Temporary or EK_CompoundLiteralInit, the type
 /// source information for the temporary.
@@ -197,9 +197,9 @@
 struct LN LocAndNRVO;
 
 /// When Kind == EK_Base, the base specifier that provides the
-/// base class. The lower bit specifies whether the base is an inherited
+/// base class. The integer specifies whether the base is an inherited
 /// virtual base.
-uintptr_t Base;
+llvm::PointerIntPair Base;
 
 /// When Kind == EK_ArrayElement, EK_VectorElement, or
 /// EK_ComplexElement, the index of the array or vector element being
@@ -252,15 +252,14 @@
 
   /// Create the initialization entity for a parameter.
   static InitializedEntity InitializeParameter(ASTContext &Context,
-   const ParmVarDecl *Parm) {
+   ParmVarDecl *Parm) {
 return InitializeParameter(Context, Parm, Parm->getType());
   }
 
   /// Create the initialization entity for a parameter, but use
   /// another type.
-  static InitializedEntity InitializeParameter(ASTContext &Context,
-   const ParmVarDecl *Parm,
-   QualType Type) {
+  static InitializedEntity
+  InitializeParameter(ASTContext &Context, ParmVarDecl *Parm, QualType Type

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman created this revision.
haberman added reviewers: rsmith, aaron.ballman.
haberman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a Clang-only change and depends on the existing "musttail"
support already implemented in LLVM.

The [[clang::musttail]] attribute goes on a return statement, not
a function definition. There are several constraints that the user
must follow when using [[clang::musttail]], and these constraints
are verified by Sema.

Tail calls are supported on regular function calls, calls through a
function pointer, member function calls, and even pointer to member.

Future work would be to throw a warning if a users tries to pass
a pointer or reference to a local variable through a musttail call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.cpp

Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int Bar();
+
+int Func1() {
+  [[clang::musttail(1, 2)]] Bar(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] Bar(); // expected-error {{musttail attribute can only be applied to a return statement}}
+  [[clang::musttail]] return 5; // expected-error {{musttail attribute requires that the return value is a function call}}
+  [[clang::musttail]] return Bar();
+}
+
+int f();
+
+[[clang::musttail]] static int i = f(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+long g(int x);
+int h(long x);
+
+class Foo {
+ public:
+  int MemberFunction(int x);
+  int MemberFunction2();
+};
+
+int Func2(int x) {
+  // Param arity differs.
+  [[clang::musttail]] return Bar(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // Return type differs.
+  [[clang::musttail]] return g(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // Param type differs.
+  [[clang::musttail]] return h(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // "this" pointer differs.
+  Foo foo;
+  [[clang::musttail]] return foo.MemberFunction(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+int j = 0;
+
+class HasNonTrivialDestructor {
+ public:
+  ~HasNonTrivialDestructor() { j--; }
+};
+
+int ReturnsInt(int x);
+
+int Func3(int x) {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsInt(x);  // expected-error {{musttail attribute does not allow any variables in scope that require destruction}}
+}
+
+int Func4(int x) {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  {
+[[clang::musttail]] return ReturnsInt(x);  // expected-error {{musttail attribute does not allow any variables in scope that require destruction}}
+  }
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+
+int Func5(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x);  // expected-error {{musttail attribute requires that the return value is a function call, which must not create or destroy any temporaries}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+
+HasNonTrivialDestructor Func6() {
+  [[clang::musttail]] return ReturnsNonTrivialValue();  // expected-error {{musttail attribute requires that the return value is a function call, which must not create or destroy any temporaries}}
+}
+
+int Func8(Foo* foo, int (Foo::*p_mem)()) {
+  [[clang::musttail]] return (foo->*p_mem)(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+int Func10(Foo* foo) {
+  [[clang::musttail]] return foo->MemberFunction2(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+void Func7() {
+  HasNonTrivialDestructor foo;
+  class Nested {
+__attribute__((noinline)) static int NestedMethod(int x) {
+  // This is ok.
+  [[clang::musttail]] return ReturnsInt(x);

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 333914.
haberman added a comment.

- Updated formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.cpp

Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int Bar();
+
+int Func1() {
+  [[clang::musttail(1, 2)]] Bar(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] Bar();   // expected-error {{musttail attribute can only be applied to a return statement}}
+  [[clang::musttail]] return 5;// expected-error {{musttail attribute requires that the return value is a function call}}
+  [[clang::musttail]] return Bar();
+}
+
+int f();
+
+[[clang::musttail]] static int i = f(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+long g(int x);
+int h(long x);
+
+class Foo {
+public:
+  int MemberFunction(int x);
+  int MemberFunction2();
+};
+
+int Func2(int x) {
+  // Param arity differs.
+  [[clang::musttail]] return Bar(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // Return type differs.
+  [[clang::musttail]] return g(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // Param type differs.
+  [[clang::musttail]] return h(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  // "this" pointer differs.
+  Foo foo;
+  [[clang::musttail]] return foo.MemberFunction(x); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+int j = 0;
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() { j--; }
+};
+
+int ReturnsInt(int x);
+
+int Func3(int x) {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsInt(x); // expected-error {{musttail attribute does not allow any variables in scope that require destruction}}
+}
+
+int Func4(int x) {
+  HasNonTrivialDestructor foo; // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  {
+[[clang::musttail]] return ReturnsInt(x); // expected-error {{musttail attribute does not allow any variables in scope that require destruction}}
+  }
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+
+int Func5(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{musttail attribute requires that the return value is a function call, which must not create or destroy any temporaries}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+
+HasNonTrivialDestructor Func6() {
+  [[clang::musttail]] return ReturnsNonTrivialValue(); // expected-error {{musttail attribute requires that the return value is a function call, which must not create or destroy any temporaries}}
+}
+
+int Func8(Foo *foo, int (Foo::*p_mem)()) {
+  [[clang::musttail]] return (foo->*p_mem)(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+int Func10(Foo *foo) {
+  [[clang::musttail]] return foo->MemberFunction2(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+}
+
+void Func7() {
+  HasNonTrivialDestructor foo;
+  class Nested {
+__attribute__((noinline)) static int NestedMethod(int x) {
+  // This is ok.
+  [[clang::musttail]] return ReturnsInt(x);
+}
+  };
+}
+
+struct Data {
+  int (Data::*pmf)();
+  typedef int Func(Data *);
+  static void StaticMethod();
+  void NonStaticMethod() {
+[[clang::musttail]] return StaticMethod(); // expected-error {{musttail attribute requires that caller and callee have identical parameter types and return types}}
+  }
+
+  void BadPMF() {
+// We need to specially handle this, otherwise it can crash the compiler.
+[[clang::musttail]] return ((*this)->*pmf)(); // expected-error {{left hand operand to ->* must be a pointer to class compatible with the right hand operand, but is 'Data'}}
+  }
+};
+
+Data data

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 334556.
haberman marked 37 inline comments as done.
haberman added a comment.

- Expanded and refined the semantic checks for musttail, per CR feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp

Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function has different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function has different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{'musttail' attribute requires that all variables in scope are trivially destructible}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return type and all arguments are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return ReturnsNonTrivialValue(); // expected-error {{'musttail' attribute requires that the return type and all arguments are trivially destructible}}
+}
+
+struct UsesPointerToMember {
+  void (UsesPointerToMember::*p_mem)();
+};
+void TestUsesPointerToMember(UsesPointerToMember *foo) {
+  // "this" pointer cannot double as first parameter.
+  [[clang::musttail]] return (foo->*(foo->p_mem))(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}} expected-note{{target function has different class (expected 'void' but has 'UsesPointerToMember')}}
+}
+
+void ReturnsVoid2();
+void TestNestedClass() {
+  HasNonTrivialDestructor foo;
+  class Nested {
+__attribute__((noinline)) static void NestedMethod() {
+  // Outer non-trivial de

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

I addressed nearly all of the comments. I have just a few more test cases to 
add: Obj-C blocks and ARC.

I left one comment open re: a "regular" function. I'll dig into that more when 
I am adding the last few test cases.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2832
+def err_musttail_only_from_function : Error<
+  "%0 attribute can only be used from a regular function.">;
+def err_musttail_return_type_mismatch : Error<

aaron.ballman wrote:
> What is a "regular function?"
I may have been trying to distinguish between blocks, or lambdas, I can't 
exactly remember.

I think I still need to add tests for blocks and Obj-C refcounts. I'm going to 
leave this comment open for now as a reminder to revisit this.



Comment at: clang/lib/CodeGen/CGCall.cpp:5315-5317
+// TODO(haberman): insert checks/assertions to verify that this early exit
+// is safe. We tried to verify this in Sema but we should double-check
+// here.

aaron.ballman wrote:
> Are you planning to handle this TODO in the patch? If not, can you switch to 
> a FIXME without a name associated with it?
I am interested in feedback on the best way to proceed here.

  - Is my assessment correct that we should have an assertion that validates 
this?
  - Is such an assertion reasonably feasible to implement?
  - Is it ok to defer with FIXME, or should I try to fix it in this patch?

I've changed it to a FIXME for now.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> This functionality belongs in SemaStmtAttr.cpp, I think.
That is where I had originally put it, but that didn't work for templates. The 
semantic checks can only be performed at instantiation time. 
`ActOnAttributedStmt` seems to be the right hook point where I can evaluate the 
semantic checks for both template and non-template functions (with template 
functions getting checked at instantiation time).



Comment at: clang/lib/Sema/SemaStmt.cpp:620
+const CXXMethodDecl *CMD = dyn_cast(mem->getMemberDecl());
+assert(CMD && !CMD->isStatic());
+CalleeThis = CMD->getThisType()->getPointeeType();

aaron.ballman wrote:
> Is this assertion valid? Consider:
> ```
> struct S {
>   static int foo();
> };
> 
> int bar() {
>   S s;
>   [[clang::musttail]] return s.foo();
> }
> ```
I have a test for that in `CodeGen/attr-musttail.cpp` (see `Func4()`). It 
appears that this goes through `FunctionToPointerDecay` and ends up getting 
handled by the function case below.



Comment at: clang/lib/Sema/SemaStmt.cpp:665-666
+ArrayRef caller_params = CallerDecl->parameters();
+size_t n = CallerDecl->param_size();
+for (size_t i = 0; i < n; i++) {
+  if (!TypesMatch(callee_params[i], caller_params[i]->getType())) {

aaron.ballman wrote:
> How do you want to handle variadic function calls?
I added a check to disallow variadic function calls.



Comment at: clang/test/Sema/attr-musttail.cpp:17
+long g(int x);
+int h(long x);
+

aaron.ballman wrote:
> I'd appreciate a test of promotion behavior:
> ```
> int i(int x);
> int s(short x);
> 
> int whatever1(int x) {
>   [[clang::musttail]] return s(x);
> }
> 
> int whatever2(short x) {
>   [[clang::musttail]] return i(x);
> }
> ```
Done (my understanding is that these should both fail, since they would violate 
the LLVM rules that the caller and callee prototypes must match).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 334890.
haberman marked 6 inline comments as done.
haberman added a comment.

- Addressed more comments for musttail.
- Reject constructors and destructors from musttail.
- Fixed a few bugs and fixed the tests.
- Added Obj-C test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,189 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fexceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{'musttail' attribute requires that all variables in scope are trivially destructible}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

I added tests for all the cases you mentioned. PTAL.




Comment at: clang/lib/CodeGen/CGCall.cpp:5315-5317
+// TODO(haberman): insert checks/assertions to verify that this early exit
+// is safe. We tried to verify this in Sema but we should double-check
+// here.

rsmith wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > Are you planning to handle this TODO in the patch? If not, can you switch 
> > > to a FIXME without a name associated with it?
> > I am interested in feedback on the best way to proceed here.
> > 
> >   - Is my assessment correct that we should have an assertion that 
> > validates this?
> >   - Is such an assertion reasonably feasible to implement?
> >   - Is it ok to defer with FIXME, or should I try to fix it in this patch?
> > 
> > I've changed it to a FIXME for now.
> Yes, I think we should validate this by an assertion if we can. We can check 
> this by walking the cleanup scope stack (walk from `CurrentCleanupScopeDepth` 
> to `EHScopeStack::stable_end()`) and making sure that there is no 
> "problematic" enclosing cleanup scope. Here, "problematic" would mean any 
> scope other than an `EHCleanupScope` containing only `CallLifetimeEnd` 
> cleanups.
> 
> Looking at the kinds of cleanups that we might encounter here, I think there 
> may be a few more things that Sema needs to check in order to not get in the 
> way of exception handling. In particular, I think we should reject if the 
> callee is potentially-throwing and the musttail call is inside a try block or 
> a function that's either noexcept or has a dynamic exception specification.
> 
> Oh, also, we should disallow musttail calls inside statement expressions, in 
> order to defend against cleanups that exist transiently within an expression.
I'm having trouble implementing the check because there doesn't appear to be 
any discriminator in `EHScopeStack::Cleanup` that will let you test if it is a 
`CallLifetimeEnd`. (The actual code just does virtual dispatch through 
`EHScopeStack::Cleanup::Emit()`.

I temporarily implemented this by adding an extra virtual function to act as 
discriminator. The check fires if a VLA is in scope:

```
int Func14(int x) {
  int vla[x];
  [[clang::musttail]] return Bar(x);
}
```

Do we need to forbid VLAs or do I need to refine the check?

It appears that `JumpDiagnostics.cpp` is already diagnosing statement 
expressions and `try`. However I could not get testing to work. I tried adding 
a test with `try`  but even with `-fexceptions` I am getting:

```
cannot use 'try' with exceptions disabled
```



Comment at: clang/lib/CodeGen/CGExpr.cpp:4828
  ReturnValueSlot ReturnValue) {
+  SaveAndRestore SaveMustTail(InMustTailCallExpr, E == MustTailCall);
+

rsmith wrote:
> The more I think about this, the more it makes me nervous: if any of the 
> `Emit*CallExpr` functions below incidentally emit a call on the way to 
> producing their results via the CGCall machinery, and do so without recursing 
> through this function, that incidental call will be emitted as a tail call 
> instead of the intended one. Specifically:
> 
> -- I could imagine a block call involving multiple function calls, depending 
> on the blocks ABI.
> -- I could imagine a member call performing a function call to convert from 
> derived to virtual base in some ABIs.
> -- A CUDA kernel call in general involves calling a setup function before the 
> actual function call happens (and it doesn't make sense for a CUDA kernel 
> call to be a tail call anyway...)
> -- A call to a builtin can result in any number of function calls.
> -- If any expression in the function arguments emits a call without calling 
> back into this function, we'll emit that call as a tail call instead of this 
> one. Eg, `[[clang::musttail]] return f(dynamic_cast(p));` might emit the 
> call to `__cxa_dynamic_cast` as the tail call instead of emitting the call to 
> `f` as the tail call, depending on whether the CGCall machinery is used when 
> emitting the `__cxa_dynamic_cast` call.
> 
> Is it feasible to sink this check into the `CodeGenFunction::EmitCall` 
> overload that takes a `CallExpr`, 
> `CodeGenFunction::EmitCXXMemberOrOperatorCall`, and 
> `CodeGenFunction::EmitCXXMemberPointerCallExpr`, after we've emitted the 
> callee and call args? It looks like we might be able to check this 
> immediately before calling the CGCall overload of `EmitCall`, so we could 
> pass in the 'musttail' information as a flag or similar instead of using 
> global state in the `CodeGenFunction` object; if so, it'd be much easier to 
> be confident that we're applying the attribute to the right call.
Done. It's feeling like `IsMustTail`, `callOrInvoke`, and `Loc` might want to 
get collapsed into an options struct, especially given the default parameters 
on the first two. Maybe could do as a follow up?



Comment

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 334985.
haberman added a comment.

- Fixed unit test by running `opt` in a separate invocation.
- Formatting fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,189 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fexceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{'musttail' attribute requires that all variables in scope are trivially destructible}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'musttail' attribute requires that 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 335103.
haberman marked 3 inline comments as done.
haberman added a comment.

- Addressed comments and tried moving check to SemaStmtAttr.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'mus

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:588
+
+  const CallExpr *CE = dyn_cast(Ex->IgnoreUnlessSpelledInSource());
+

rsmith wrote:
> `IgnoreUnlessSpelledInSource` is a syntactic check that's only really 
> intended for tooling use cases; I think we want something a bit more semantic 
> here, so `IgnoreImplicitAsWritten` would be more appropriate.
> 
> I think it would be reasonable to also skip "parentheses" here (which we 
> treat as also including things like C's `_Generic`). Would 
> `Ex->IgnoreImplicitAsWritten()->IgnoreParens()` work?
> 
> If we're going to skip elidable copy construction of the result here (which I 
> think we should), should we also reflect that in the AST? Perhaps we should 
> strip the return value down to being just the call expression? I'm thinking 
> in particular of things like building in C++14 or before with 
> `-fno-elide-constructors`, where code generation for a by-value return of a 
> class object will synthesize a local temporary to hold the result, with a 
> final destination copy emitted after the call. (Testcase: `struct A { A(const 
> A&); }; A f(); A g() { [[clang::musttail]] return f(); }` with 
> `-fno-elide-constructors`.)
`IgnoreImplicitAsWritten()` doesn't skip `ExprWithCleanups`, and per your 
previous comment I was trying to find a `CallExpr` before doing the check 
prohibiting `ExprWithCleanups` with side effects.

I could write some custom ignore logic using `clang::IgnoreExprNodes()` 
directly.

> If we're going to skip elidable copy construction of the result here (which I 
> think we should)

To clarify, are you suggesting that we allow `musttail` through elidable copy 
constructors on the return value, even if `-fno-elide-constructors` is set? ie. 
we consider that `musttail` overrides the `-fno-elide-constructors` option on 
the command line?



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > That is where I had originally put it, but that didn't work for templates. 
> > The semantic checks can only be performed at instantiation time. 
> > `ActOnAttributedStmt` seems to be the right hook point where I can evaluate 
> > the semantic checks for both template and non-template functions (with 
> > template functions getting checked at instantiation time).
> I disagree that `ActOnAttributedStmt()` is the correct place for this 
> checking -- template checking should occur when the template is instantiated, 
> same as happens for declaration attributes. I'd like to see this 
> functionality moved to SemaStmtAttr.cpp. Keeping the attribute logic together 
> and following the same patterns is what allows us to tablegenerate more of 
> the attribute logic. Statement attributes are just starting to get more such 
> automation.
I tried commenting out this code and adding the following code into 
`handleMustTailAttr()` in `SemaStmtAttr.cpp`:

```
  if (!S.checkMustTailAttr(St, MTA))
return nullptr;
```

This caused my test cases related to templates to fail. It also seemed to break 
test cases related to `JumpDiagnostics`. My interpretation of this is that 
`handleMustTailAttr()` is called during parsing only, and cannot catch errors 
at template instantiation time or that require a more complete AST.

What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I put this 
check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 335106.
haberman added a comment.

- Added missing S.setFunctionHasMustTail().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from Objective-C blocks}}
+  };
+  __attribute__((musttail)) return x();
+}
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible and do not require ARC}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'musttail' attribute requires that the return value, all parameters,

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

haberman wrote:
> aaron.ballman wrote:
> > haberman wrote:
> > > aaron.ballman wrote:
> > > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > > That is where I had originally put it, but that didn't work for 
> > > templates. The semantic checks can only be performed at instantiation 
> > > time. `ActOnAttributedStmt` seems to be the right hook point where I can 
> > > evaluate the semantic checks for both template and non-template functions 
> > > (with template functions getting checked at instantiation time).
> > I disagree that `ActOnAttributedStmt()` is the correct place for this 
> > checking -- template checking should occur when the template is 
> > instantiated, same as happens for declaration attributes. I'd like to see 
> > this functionality moved to SemaStmtAttr.cpp. Keeping the attribute logic 
> > together and following the same patterns is what allows us to tablegenerate 
> > more of the attribute logic. Statement attributes are just starting to get 
> > more such automation.
> I tried commenting out this code and adding the following code into 
> `handleMustTailAttr()` in `SemaStmtAttr.cpp`:
> 
> ```
>   if (!S.checkMustTailAttr(St, MTA))
> return nullptr;
> ```
> 
> This caused my test cases related to templates to fail. It also seemed to 
> break test cases related to `JumpDiagnostics`. My interpretation of this is 
> that `handleMustTailAttr()` is called during parsing only, and cannot catch 
> errors at template instantiation time or that require a more complete AST.
> 
> What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I put 
> this check?
Scratch the part about `JumpDiagnostics`, that was me failing to call 
`S.setFunctionHasMustTail()`. I added that and now the `JumpDiagnostics` tests 
pass.

But the template test cases still fail, and I can't find any hook point in 
`SemaStmtAttr.cpp` that will let me evaluate these checks at template 
instantiation time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > haberman wrote:
> > > aaron.ballman wrote:
> > > > haberman wrote:
> > > > > aaron.ballman wrote:
> > > > > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > > > > That is where I had originally put it, but that didn't work for 
> > > > > templates. The semantic checks can only be performed at instantiation 
> > > > > time. `ActOnAttributedStmt` seems to be the right hook point where I 
> > > > > can evaluate the semantic checks for both template and non-template 
> > > > > functions (with template functions getting checked at instantiation 
> > > > > time).
> > > > I disagree that `ActOnAttributedStmt()` is the correct place for this 
> > > > checking -- template checking should occur when the template is 
> > > > instantiated, same as happens for declaration attributes. I'd like to 
> > > > see this functionality moved to SemaStmtAttr.cpp. Keeping the attribute 
> > > > logic together and following the same patterns is what allows us to 
> > > > tablegenerate more of the attribute logic. Statement attributes are 
> > > > just starting to get more such automation.
> > > I tried commenting out this code and adding the following code into 
> > > `handleMustTailAttr()` in `SemaStmtAttr.cpp`:
> > > 
> > > ```
> > >   if (!S.checkMustTailAttr(St, MTA))
> > > return nullptr;
> > > ```
> > > 
> > > This caused my test cases related to templates to fail. It also seemed to 
> > > break test cases related to `JumpDiagnostics`. My interpretation of this 
> > > is that `handleMustTailAttr()` is called during parsing only, and cannot 
> > > catch errors at template instantiation time or that require a more 
> > > complete AST.
> > > 
> > > What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I 
> > > put this check?
> > Scratch the part about `JumpDiagnostics`, that was me failing to call 
> > `S.setFunctionHasMustTail()`. I added that and now the `JumpDiagnostics` 
> > tests pass.
> > 
> > But the template test cases still fail, and I can't find any hook point in 
> > `SemaStmtAttr.cpp` that will let me evaluate these checks at template 
> > instantiation time.
> I think there's a bit of an architectural mixup, but I'm curious if @rsmith 
> agrees before anyone starts doing work to make changes.
> 
> When transforming declarations, `RebuildWhatever()` calls the 
> `ActOnWhatever()` function which calls `ProcessDeclAttributeList()` so that 
> attributes are processed. `RebuildAttributedStmt()` similarly calls 
> `ActOnAttributedStmt()`. However, `ActOnAttributedStmt()` doesn't call 
> `ProcessStmtAttributes()` -- the logic is reversed so that 
> `ProcessStmtAttributes()` is what calls `ActOnAttributedStmt()`.
> 
> I think the correct answer is to switch the logic so that 
> `ActOnAttributedStmt()` calls `ProcessStmtAttributes()`, then the template 
> logic should automatically work.
> I think the correct answer is to switch the logic so that 
> ActOnAttributedStmt() calls ProcessStmtAttributes()

I think this would require `ProcessStmtAttributes()` to be split into two 
separate functions. Currently that function is doing two separate things:

1. Translation of `ParsedAttr` into various subclasses of `Attr`.
2. Validation that the attribute is semantically valid.

The function signature for `ActOnAttributedStmt()` uses `Attr` (not 
`ParsedAttr`), so (1) must happen during the parse, before 
`ActOnAttributedStmt()` is called. But (2) must be deferred until template 
instantiation time for some cases, like `musttail`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99896: Rework the way statement attributes are processed; NFC

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:1316
Stmt *SubStmt) {
-return SemaRef.ActOnAttributedStmt(AttrLoc, Attrs, SubStmt);
+return SemaRef.BuildAttributedStmt(AttrLoc, Attrs, SubStmt);
   }

aaron.ballman wrote:
> erichkeane wrote:
> > Am I missing where the attributes themselves are being 
> > rebuilt/transformed??  
> > 
> > 
> The transformation happens in 
> `TreeTransform::TransformAttributedStmt()` which calls 
> `RebuildAttributedStmt()` with the rebuilt attributes.
It appears that neither `TransformAttributedStmt()` nor 
`RebuildAttributedStmt()` calls `ProcessStmtAttributes()`, either directly or 
transitively, so I'm not seeing where we can run instantiation-time attribute 
processing logic. What am I missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99896

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

[PATCH] D99896: Rework the way statement attributes are processed; NFC

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:1316
Stmt *SubStmt) {
-return SemaRef.ActOnAttributedStmt(AttrLoc, Attrs, SubStmt);
+return SemaRef.BuildAttributedStmt(AttrLoc, Attrs, SubStmt);
   }

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > Am I missing where the attributes themselves are being 
> > > > rebuilt/transformed??  
> > > > 
> > > > 
> > > The transformation happens in 
> > > `TreeTransform::TransformAttributedStmt()` which calls 
> > > `RebuildAttributedStmt()` with the rebuilt attributes.
> > It appears that neither `TransformAttributedStmt()` nor 
> > `RebuildAttributedStmt()` calls `ProcessStmtAttributes()`, either directly 
> > or transitively, so I'm not seeing where we can run instantiation-time 
> > attribute processing logic. What am I missing?
> My thinking is: 
> 
> * From `handleMustTailAttr()` in SemaStmtAttr.cpp, call `CheckMustTailAttr()` 
> to do the shared semantic checking.
> * Add a `TransformMustTailAttr()` to `TreeTransform`, have it call 
> `SemaRef.CheckMustTailAttr()` as well.
I see. My main concern then is that `TransformMustTailAttr()` could get access 
to the `ReturnExpr` to perform the validation. Right now the `MustTailAttr` 
doesn't appear to have any reference to the `ReturnExpr`, and I don't know how 
to give it one.

If there is a solution to this problem, I don't have any objection. My main 
concern is to unblock my change which is a high priority for me and my team.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99896

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > haberman wrote:
> > > > > aaron.ballman wrote:
> > > > > > haberman wrote:
> > > > > > > haberman wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > haberman wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > This functionality belongs in SemaStmtAttr.cpp, I think.
> > > > > > > > > > That is where I had originally put it, but that didn't work 
> > > > > > > > > > for templates. The semantic checks can only be performed at 
> > > > > > > > > > instantiation time. `ActOnAttributedStmt` seems to be the 
> > > > > > > > > > right hook point where I can evaluate the semantic checks 
> > > > > > > > > > for both template and non-template functions (with template 
> > > > > > > > > > functions getting checked at instantiation time).
> > > > > > > > > I disagree that `ActOnAttributedStmt()` is the correct place 
> > > > > > > > > for this checking -- template checking should occur when the 
> > > > > > > > > template is instantiated, same as happens for declaration 
> > > > > > > > > attributes. I'd like to see this functionality moved to 
> > > > > > > > > SemaStmtAttr.cpp. Keeping the attribute logic together and 
> > > > > > > > > following the same patterns is what allows us to 
> > > > > > > > > tablegenerate more of the attribute logic. Statement 
> > > > > > > > > attributes are just starting to get more such automation.
> > > > > > > > I tried commenting out this code and adding the following code 
> > > > > > > > into `handleMustTailAttr()` in `SemaStmtAttr.cpp`:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > >   if (!S.checkMustTailAttr(St, MTA))
> > > > > > > > return nullptr;
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > This caused my test cases related to templates to fail. It also 
> > > > > > > > seemed to break test cases related to `JumpDiagnostics`. My 
> > > > > > > > interpretation of this is that `handleMustTailAttr()` is called 
> > > > > > > > during parsing only, and cannot catch errors at template 
> > > > > > > > instantiation time or that require a more complete AST.
> > > > > > > > 
> > > > > > > > What am I missing? Where in SemaStmtAttr.cpp are you suggesting 
> > > > > > > > that I put this check?
> > > > > > > Scratch the part about `JumpDiagnostics`, that was me failing to 
> > > > > > > call `S.setFunctionHasMustTail()`. I added that and now the 
> > > > > > > `JumpDiagnostics` tests pass.
> > > > > > > 
> > > > > > > But the template test cases still fail, and I can't find any hook 
> > > > > > > point in `SemaStmtAttr.cpp` that will let me evaluate these 
> > > > > > > checks at template instantiation time.
> > > > > > I think there's a bit of an architectural mixup, but I'm curious if 
> > > > > > @rsmith agrees before anyone starts doing work to make changes.
> > > > > > 
> > > > > > When transforming declarations, `RebuildWhatever()` calls the 
> > > > > > `ActOnWhatever()` function which calls `ProcessDeclAttributeList()` 
> > > > > > so that attributes are processed. `RebuildAttributedStmt()` 
> > > > > > similarly calls `ActOnAttributedStmt()`. However, 
> > > > > > `ActOnAttributedStmt()` doesn't call `ProcessStmtAttributes()` -- 
> > > > > > the logic is reversed so that `ProcessStmtAttributes()` is what 
> > > > > > calls `ActOnAttributedStmt()`.
> > > > > > 
> > > > > > I think the correct answer is to switch the logic so that 
> > > > > > `ActOnAttributedStmt()` calls `ProcessStmtAttributes()`, then the 
> > > > > > template logic should automatically work.
> > > > > > I think the correct answer is to switch the logic so that 
> > > > > > ActOnAttributedStmt() calls ProcessStmtAttributes()
> > > > > 
> > > > > I think this would require `ProcessStmtAttributes()` to be split into 
> > > > > two separate functions. Currently that function is doing two separate 
> > > > > things:
> > > > > 
> > > > > 1. Translation of `ParsedAttr` into various subclasses of `Attr`.
> > > > > 2. Validation that the attribute is semantically valid.
> > > > > 
> > > > > The function signature for `ActOnAttributedStmt()` uses `Attr` (not 
> > > > > `ParsedAttr`), so (1) must happen during the parse, before 
> > > > > `ActOnAttributedStmt()` is called. But (2) must be deferred until 
> > > > > template instantiation time for some cases, like `musttail`.
> > > > I don't think the signature for `ActOnAttributedStmt()` is correct to 
> > > > use `Attr` instead of `ParsedAttr`. I think it should be `StmtResult 
> > > > ActOnAttributedStmt(const ParsedAttributesViewWithRange &AttrList, Stmt 
> > > > *SubStmt);` -- this likely requires a fair bit of s

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336004.
haberman marked 19 inline comments as done.
haberman added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

- Returned validation to ActOnAttributedStmt() so it works with templates.
- Merge branch 'main' into musttail
- Address more review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid();  // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+//#import 
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,191 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return;  // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+ 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:458
+same number of arguments as the caller. The types of the return value and all
+arguments must be similar, including the implicit "this" argument, if any.
+Any variables in scope, including all arguments to the function and the

aaron.ballman wrote:
> It'd be nice if we could nail down "similar" somewhat. I don't know if `int` 
> and `short` are similar (due to promotions) or `const int` and `int` are 
> similar, etc.
Done. I tried to summarize the C++ concept of "similar" types as defined in 
https://eel.is/c++draft/conv.qual#2 and implemented in 
https://clang.llvm.org/doxygen/classclang_1_1ASTContext.html#a1b1b3b7a67a30fd817ba85454780d8ad



Comment at: clang/lib/Sema/SemaStmt.cpp:596
+  if (!CE->getCalleeDecl()) {
+assert(hasUncompilableErrorOccurred() && "expected previous error");
+return false;

rsmith wrote:
> A call expression doesn't necessarily have a known callee declaration. I 
> would expect this assert to fire on a case like:
> ```
> void f() {
>   void (*p)() = f;
>   [[clang::musttail]] return p();
> }
> ```
> We should reject this with a diagnostic.
I think this case will work actually, the callee decl in this case is just the 
function pointer, which seems appropriate and type checks correctly.

I added a test for this.



Comment at: clang/lib/Sema/SemaStmt.cpp:631
+// Caller is an Obj-C block decl: ^(void) { /* ... */ }
+assert(dyn_cast(CurContext) && "unexpected decl context");
+Diag(St->getBeginLoc(), diag::err_musttail_from_block_forbidden) << &MTA;

rjmccall wrote:
> rsmith wrote:
> > There are a couple of other contexts that can include a return statement: 
> > the caller could also be an `ObjCMethodDecl` (an Objective-C method) or a 
> > `CapturedDecl` (the body of a `#pragma omp` parallel region). I'd probably 
> > use a specific diagnostic ("cannot be used from a block" / "cannot be used 
> > from an Objective-C function") for the block and ObjCMethod case, and a 
> > nonsepcific-but-correct "cannot be used from this context" for anything 
> > else.
> Blocks ought to be extremely straightforward to support.  Just validate that 
> the tail call is to a block pointer and then compare the underlying function 
> types line up in the same way.  You will need to be able to verify that there 
> isn't a non-trivial conversion on the return types, even if the return type 
> isn't known at this point in the function, but that's a problem in C++ as 
> well due to lambdas and `auto` deduced return types.
> 
> Also, you can use `isa<...>` for checks like this instead of `dyn_cast<...>`.
Tail calls to a block are indeed straightforward and are handled below. This 
check is for tail calls from a block, which I tried to add support for but 
didn't have much luck (in particular, during parsing of a block I wasn't able 
to get good type information for the block).

> I'd probably use a specific diagnostic ("cannot be used from a block" / 
> "cannot be used from an Objective-C function") for the block and ObjCMethod 
> case, and a nonsepcific-but-correct "cannot be used from this context" for 
> anything else.

I implemented this as requested. I wasn't able to test OpenMP as you apparently 
can't return from an OpenMP block.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > rsmith wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > haberman wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > haberman wrote:
> > > > > > > > > haberman wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > This functionality belongs in SemaStmtAttr.cpp, I 
> > > > > > > > > > > > > think.
> > > > > > > > > > > > That is where I had originally put it, but that didn't 
> > > > > > > > > > > > work for templates. The semantic checks can only be 
> > > > > > > > > > > > performed at instantiation time. `ActOnAttributedStmt` 
> > > > > > > > > > > > seems to be the right hook point where I can evaluate 
> > > > > > > > > > > > the semantic checks for both template and non-template 
> > > > > > > > > > > > functions (with template functions getting checked at 
> > > > > > > > > > > > instantiation time).
> > > > > > > > > > > I disagree that `ActOnAttributedStmt()` is the correct 
> > > > > > > > > > > place for this checking -- template checking should occur 
> > > > > > > > > > > when the template is instantiated, same as happens for 
> > > > > > > > > > > declaration attributes. I'd like to

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336130.
haberman added a comment.

- Added FIXME for attribute refactoring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid();  // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+//#import 
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,191 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return;  // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo; 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336141.
haberman added a comment.

- Factored duplicated code into a method on MustTailAttr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid();  // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+//#import 
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,191 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return;  // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > haberman wrote:
> > > > aaron.ballman wrote:
> > > > > rsmith wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > haberman wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > haberman wrote:
> > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > This functionality belongs in SemaStmtAttr.cpp, I 
> > > > > > > > > > > > > > > think.
> > > > > > > > > > > > > > That is where I had originally put it, but that 
> > > > > > > > > > > > > > didn't work for templates. The semantic checks can 
> > > > > > > > > > > > > > only be performed at instantiation time. 
> > > > > > > > > > > > > > `ActOnAttributedStmt` seems to be the right hook 
> > > > > > > > > > > > > > point where I can evaluate the semantic checks for 
> > > > > > > > > > > > > > both template and non-template functions (with 
> > > > > > > > > > > > > > template functions getting checked at instantiation 
> > > > > > > > > > > > > > time).
> > > > > > > > > > > > > I disagree that `ActOnAttributedStmt()` is the 
> > > > > > > > > > > > > correct place for this checking -- template checking 
> > > > > > > > > > > > > should occur when the template is instantiated, same 
> > > > > > > > > > > > > as happens for declaration attributes. I'd like to 
> > > > > > > > > > > > > see this functionality moved to SemaStmtAttr.cpp. 
> > > > > > > > > > > > > Keeping the attribute logic together and following 
> > > > > > > > > > > > > the same patterns is what allows us to tablegenerate 
> > > > > > > > > > > > > more of the attribute logic. Statement attributes are 
> > > > > > > > > > > > > just starting to get more such automation.
> > > > > > > > > > > > I tried commenting out this code and adding the 
> > > > > > > > > > > > following code into `handleMustTailAttr()` in 
> > > > > > > > > > > > `SemaStmtAttr.cpp`:
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > >   if (!S.checkMustTailAttr(St, MTA))
> > > > > > > > > > > > return nullptr;
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > This caused my test cases related to templates to fail. 
> > > > > > > > > > > > It also seemed to break test cases related to 
> > > > > > > > > > > > `JumpDiagnostics`. My interpretation of this is that 
> > > > > > > > > > > > `handleMustTailAttr()` is called during parsing only, 
> > > > > > > > > > > > and cannot catch errors at template instantiation time 
> > > > > > > > > > > > or that require a more complete AST.
> > > > > > > > > > > > 
> > > > > > > > > > > > What am I missing? Where in SemaStmtAttr.cpp are you 
> > > > > > > > > > > > suggesting that I put this check?
> > > > > > > > > > > Scratch the part about `JumpDiagnostics`, that was me 
> > > > > > > > > > > failing to call `S.setFunctionHasMustTail()`. I added 
> > > > > > > > > > > that and now the `JumpDiagnostics` tests pass.
> > > > > > > > > > > 
> > > > > > > > > > > But the template test cases still fail, and I can't find 
> > > > > > > > > > > any hook point in `SemaStmtAttr.cpp` that will let me 
> > > > > > > > > > > evaluate these checks at template instantiation time.
> > > > > > > > > > I think there's a bit of an architectural mixup, but I'm 
> > > > > > > > > > curious if @rsmith agrees before anyone starts doing work 
> > > > > > > > > > to make changes.
> > > > > > > > > > 
> > > > > > > > > > When transforming declarations, `RebuildWhatever()` calls 
> > > > > > > > > > the `ActOnWhatever()` function which calls 
> > > > > > > > > > `ProcessDeclAttributeList()` so that attributes are 
> > > > > > > > > > processed. `RebuildAttributedStmt()` similarly calls 
> > > > > > > > > > `ActOnAttributedStmt()`. However, `ActOnAttributedStmt()` 
> > > > > > > > > > doesn't call `ProcessStmtAttributes()` -- the logic is 
> > > > > > > > > > reversed so that `ProcessStmtAttributes()` is what calls 
> > > > > > > > > > `ActOnAttributedStmt()`.
> > > > > > > > > > 
> > > > > > > > > > I think the correct answer is to switch the logic so that 
> > > > > > > > > > `ActOnAttributedStmt()` calls `ProcessStmtAttributes()`, 
> > > > > > > > > > then the template logic should automatically work.
> > > > > > > > > > I think the correct answer is to switch the logic so that 
> > > > > > > > > > ActOnAttributedStmt() calls ProcessStmtAttributes()
> > > > > > > > > 
> > > > > > > > > I think this would require `ProcessStmtAttributes()` to be 
> > > > > > > > > split into two separ

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336153.
haberman added a comment.

- Moved calling convention check to happen as early as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid();  // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+//#import 
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,191 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();   // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;// expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return;  // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInS

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

In D99517#2667025 , @rjmccall wrote:

> You should structure this code so it's easy to add exceptions for certain 
> calling conventions that can support tail calls with weaker restrictions 
> (principally, callee-pop conventions).  Mostly that probably means checking 
> the calling convention first, or extracting the type restriction checks into 
> a different function that you can skip.  For example, I believe x86's 
> `fastcall` convention can logically support any combination of prototypes as 
> `musttail` as long as the return types are vaguely compatible.

I moved the calling convention check to be as early as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336203.
haberman added a comment.

- Formatted files with clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+//#import 
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,191 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNon

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman marked 2 inline comments as done.
haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:561-568
+  for (const auto *A : Attrs) {
+if (A->getKind() == attr::MustTail) {
+  if (!checkMustTailAttr(SubStmt, *A)) {
+return SubStmt;
+  }
+  setFunctionHasMustTail();
+}

aaron.ballman wrote:
> haberman wrote:
> > aaron.ballman wrote:
> > > haberman wrote:
> > > > aaron.ballman wrote:
> > > > > haberman wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > rsmith wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > haberman wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > haberman wrote:
> > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > This functionality belongs in 
> > > > > > > > > > > > > > > > > SemaStmtAttr.cpp, I think.
> > > > > > > > > > > > > > > > That is where I had originally put it, but that 
> > > > > > > > > > > > > > > > didn't work for templates. The semantic checks 
> > > > > > > > > > > > > > > > can only be performed at instantiation time. 
> > > > > > > > > > > > > > > > `ActOnAttributedStmt` seems to be the right 
> > > > > > > > > > > > > > > > hook point where I can evaluate the semantic 
> > > > > > > > > > > > > > > > checks for both template and non-template 
> > > > > > > > > > > > > > > > functions (with template functions getting 
> > > > > > > > > > > > > > > > checked at instantiation time).
> > > > > > > > > > > > > > > I disagree that `ActOnAttributedStmt()` is the 
> > > > > > > > > > > > > > > correct place for this checking -- template 
> > > > > > > > > > > > > > > checking should occur when the template is 
> > > > > > > > > > > > > > > instantiated, same as happens for declaration 
> > > > > > > > > > > > > > > attributes. I'd like to see this functionality 
> > > > > > > > > > > > > > > moved to SemaStmtAttr.cpp. Keeping the attribute 
> > > > > > > > > > > > > > > logic together and following the same patterns is 
> > > > > > > > > > > > > > > what allows us to tablegenerate more of the 
> > > > > > > > > > > > > > > attribute logic. Statement attributes are just 
> > > > > > > > > > > > > > > starting to get more such automation.
> > > > > > > > > > > > > > I tried commenting out this code and adding the 
> > > > > > > > > > > > > > following code into `handleMustTailAttr()` in 
> > > > > > > > > > > > > > `SemaStmtAttr.cpp`:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > >   if (!S.checkMustTailAttr(St, MTA))
> > > > > > > > > > > > > > return nullptr;
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > This caused my test cases related to templates to 
> > > > > > > > > > > > > > fail. It also seemed to break test cases related to 
> > > > > > > > > > > > > > `JumpDiagnostics`. My interpretation of this is 
> > > > > > > > > > > > > > that `handleMustTailAttr()` is called during 
> > > > > > > > > > > > > > parsing only, and cannot catch errors at template 
> > > > > > > > > > > > > > instantiation time or that require a more complete 
> > > > > > > > > > > > > > AST.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > What am I missing? Where in SemaStmtAttr.cpp are 
> > > > > > > > > > > > > > you suggesting that I put this check?
> > > > > > > > > > > > > Scratch the part about `JumpDiagnostics`, that was me 
> > > > > > > > > > > > > failing to call `S.setFunctionHasMustTail()`. I added 
> > > > > > > > > > > > > that and now the `JumpDiagnostics` tests pass.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > But the template test cases still fail, and I can't 
> > > > > > > > > > > > > find any hook point in `SemaStmtAttr.cpp` that will 
> > > > > > > > > > > > > let me evaluate these checks at template 
> > > > > > > > > > > > > instantiation time.
> > > > > > > > > > > > I think there's a bit of an architectural mixup, but 
> > > > > > > > > > > > I'm curious if @rsmith agrees before anyone starts 
> > > > > > > > > > > > doing work to make changes.
> > > > > > > > > > > > 
> > > > > > > > > > > > When transforming declarations, `RebuildWhatever()` 
> > > > > > > > > > > > calls the `ActOnWhatever()` function which calls 
> > > > > > > > > > > > `ProcessDeclAttributeList()` so that attributes are 
> > > > > > > > > > > > processed. `RebuildAttributedStmt()` similarly calls 
> > > > > > > > > > > > `ActOnAttributedStmt()`. However, 
> > > > > > > > > > > > `ActOnAttributedStmt()` doesn't call 
> > > > > > > > > > > > `ProcessStmtAttributes()` -- the logic is reversed so 
> > > > > > > > > > > > that `ProcessStmtAttributes()` is what calls 
> > > > > > > > > > > > `ActOnAttributedStmt()`.
> > > > > > > > > > > > 
> > > > > > > > > > > > I think 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336310.
haberman marked 3 inline comments as done.
haberman added a comment.

- Refined the implicit constructor skipping code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,201 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivia

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2828-2829
+  "%0 attribute requires that the return value, all parameters, and any "
+  "temporaries created by the expression are trivially destructible and "
+  "do not require ARC">;
+def err_musttail_needs_call : Error<

rsmith wrote:
> Can we somehow avoid talking about ARC where it's not relevant? While it'd be 
> nice to be more precise here, my main concern is that we shouldn't be 
> mentioning ARC to people for whom it's not a meaningful term (eg, when not 
> compiling Objective-C or Objective-C++). Perhaps the simplest approach would 
> be to only mention ARC if `getLangOpts().ObjCAutoRefCount` is set?
I implemented this but I couldn't figure out how to actually trigger the ARC 
case, so I just removed that part of the diagnostic text for now.



Comment at: clang/lib/AST/AttrImpl.cpp:221-226
+  // Elide constructors even if this is disabled with -fno-elide-constructors
+  // because 'musttail' is a more local setting.
+  if (CCE && CCE->isElidable()) {
+assert(CCE->getNumArgs() == 1); // Copy or move constructor.
+Ex = CCE->getArg(0);
+  }

rsmith wrote:
> `IgnoreImplicitAsWritten` should already skip over implicit elidable 
> constructors, so I would imagine this is skipping over elidable //explicit// 
> constructor calls (eg, `[[musttail]] return T(make());` would perform a 
> tail-call to `make()`). Is that what we want?
As discussed offline, it appears that `IgnoreImplicitAsWritten()` was not 
skipping the implicit constructor in this case. Per our discussion, I created a 
new version of `IgnoreImplicitAsWritten()` that does, with a FIXME to land it 
in `Expr`, and I made it skip implicit constructors only (and added tests for 
this case).



Comment at: clang/lib/CodeGen/CGStmt.cpp:665
+  SaveAndRestore save_musttail(MustTailCall, musttail);
   EmitStmt(S.getSubStmt(), S.getAttrs());
 }

rsmith wrote:
> In the case where we're forcibly eliding a constructor, we'll need to emit a 
> return statement that returns `musttail` call expression here rather than 
> emitting the original substatement. Otherwise the tail call we emit will be 
> initializing a local temporary rather than initializing our return slot. Eg, 
> given:
> 
> ```
> struct A {
>   A(const A&);
>   ~A();
>   char data[32];
> };
> A f();
> A g() {
>   [[clang::musttail]] return f();
> }
> ```
> under `-fno-elide-constructors` when targeting C++11, say, we'll normally 
> lower that into something like:
> ```
> void f(A *return_slot);
> void g(A *return_slot) {
>   A temporary; //uninitialized
>   f(&temporary); // call f
>   A::A(return_slot, temporary); // call copy constructor to copy into return 
> slot
> }
> ```
> ... and with the current patch, it looks like we'll add a 'ret void' after 
> the call to `f`, leaving `g`'s return slot uninitialized and passing an 
> address into `f` that refers to a variable that will no longer exist once `f` 
> is called. We need to instead lower to:
> ```
> void f(A *return_slot);
> void g(A *return_slot) {
>   f(return_slot); // call f
> }
> ```
> Probably the easiest way to do this would be to change the return value on 
> the `ReturnStmt` to be the tail-called `CallExpr` when attaching the 
> attribute.
Done.

I had to change your test case to remove the destructor, otherwise it fails the 
trivial destruction check.

Take a look at the CodeGen tests and see if the output looks correct to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336316.
haberman added a comment.

- Rename and refine IgnoreElidableImplicitConstructorSingleStep().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,201 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336511.
haberman marked 9 inline comments as done.
haberman added a comment.

- Simplified some casts and type declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,201 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDe

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:636-637
+
+  if (!CE->getCalleeDecl()) {
+assert(hasUncompilableErrorOccurred() && "expected previous error");
+return false;

aaron.ballman wrote:
> This worries me slightly -- not all `CallExpr` objects have a callee 
> declaration 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/Expr.cpp#L1367).
>  That said, I'm struggling to come up with an example that isn't covered so 
> this may be fine.
That was my experience too, I wasn't able to find a case that isn't covered. I 
tried to avoid adding any diagnostics that I didn't know how to trigger or test.



Comment at: clang/lib/Sema/SemaStmt.cpp:700
+// Call is: obj->*method_ptr or obj.*method_ptr
+const MemberPointerType *MPT = VD->getType()->castAs();
+CalleeType.This = QualType(MPT->getClass(), 0);

aaron.ballman wrote:
> It'd be better not to go through the cast machinery twice -- you cast to the 
> `MemberPointerType` and then cast to the same thing again (but in a different 
> way).
I changed to `auto`, but I can't tell if you have another suggestion here also. 
I can't see how any of these casts can be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 336894.
haberman added a comment.

- Switch to isa<> for type check.
- Merge branch 'main' into musttail


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.cpp
  clang/test/Sema/attr-musttail.m

Index: clang/test/Sema/attr-musttail.m
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.m
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class -verify %s
+
+void TestObjcBlock(void) {
+  void (^x)(void) = ^(void) {
+__attribute__((musttail)) return TestObjcBlock(); // expected-error{{'musttail' attribute cannot be used from a block}}
+  };
+  __attribute__((musttail)) return x();
+}
+
+void ReturnsVoid(void);
+void TestObjcBlockVar(void) {
+  __block int i = 0;  // expected-note{{jump exits scope of __block variable}}
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{cannot perform a tail call from this return statement}}
+}
+
+__attribute__((objc_root_class))
+@interface TestObjcClass
+@end
+
+@implementation TestObjcClass
+
+- (void)testObjCMethod {
+  __attribute__((musttail)) return ReturnsVoid(); // expected-error{{'musttail' attribute cannot be used from an Objective-C function}}
+}
+
+@end
Index: clang/test/Sema/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-musttail.cpp
@@ -0,0 +1,201 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member of different class (expected 'void' but has 'Struct1')}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is a member of different class (expected 'Struct2' but has 'void')}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{'musttail' attribute requires that caller and callee have compatible function signatures}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman marked an inline comment as done.
haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:697-699
+  } else if (VD && dyn_cast(VD->getType())) {
+// Call is: obj->*method_ptr or obj.*method_ptr
+const auto *MPT = VD->getType()->castAs();

aaron.ballman wrote:
> I'm not certain if I should take a shower after writing that code or not, but 
> it's one potential way not to perform the cast twice.
> 
> If that code is too odious for others, we should at least change the 
> `dyn_cast<>` in the `else if` to be an `isa<>`.
I changed `dyn_cast<>` to `isa<>`. If @rsmith concurs about the 
`dyn_cast_or_null<>` variant I'll switch to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337252.
haberman marked 14 inline comments as done.
haberman added a comment.

- Addressed more review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,227 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] return NoParams(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]] return LongParam(x); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]] return ReturnsLong(); // expected-error {{'musttail' attribute requires that caller and callee have compatible function signatures}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{target function is a member function of class 'Struct1'}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] return st.MemberFunction(); // expected-error {{cannot 'musttail' from a non-member to a member function}}
+}
+
+void ReturnsVoid(); // expected-note {{target function is not a member function}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]] return ReturnsVoid(); // expected-error{{cannot 'musttail' from a member to a non-member function}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the diagnostic cannot currently distinguish between needing to run a
+  // destructor for the return value and needing to run a destructor for some
+  // other temporary created in the return statement.
+  [[clang::musttail]] return (ReturnsNonTrivialValue()); // expected-error {{'musttail' attribute requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor TestReturnsNonTrivialNon

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:605-609
+  while (!isa(Ex)) {
+auto *PE = cast(Ex);
+Ex = IgnoreImplicitAsWritten(PE->getSubExpr());
+PE->setSubExpr(Ex);
+  }

rsmith wrote:
> This loop is problematic: it's generally not safe to modify an expression 
> that is used as a subexpression of another expression. (Modifying the 
> `ReturnStmt` is, by contrast, much less problematic because the properties of 
> a statement have less complex dependencies on the properties of its 
> subexpressions.) In particular, if there were any implicit conversions here 
> that changed the type or value category or similar, the enclosing parentheses 
> would have the wrong type / value category / similar. Also there are 
> possibilities here other than `CallExpr` and `ParenExpr`, such as anything 
> else that we consider to be "parentheses" (such as a `GenericSelectionExpr`).
> 
> But I think this loop should never be necessary, because all implicit 
> conversions should always be on the outside of the parentheses. Do you have a 
> testcase that needs it?
I removed it and my test cases still pass. I'm glad to know this isn't 
necessary: I was coding defensively because I didn't know that I could count on 
this invariant:

> all implicit conversions should always be on the outside of the parentheses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:711
+if (CalleeQualType.isNull()) {
+  // The function callee is invalid and already triggered an error.
+  // Avoid compounding errors.

rsmith wrote:
> Even in invalid code we should never see a `CallExpr` whose callee has a null 
> type; if `Sema` can't form an `Expr` that meets the normal expression 
> invariants during error recovery, it doesn't build one at all. I think you 
> can remove this `if`.
Without this if(), I crash on this test case. What do you think?

```
struct TestBadPMF {
  int (TestBadPMF::*pmf)();
  void BadPMF() {
[[clang::musttail]] return ((*this)->*pmf)(); // expected-error {{left hand 
operand to ->* must be a pointer to class compatible with the right hand 
operand, but is 'TestBadPMF'}}
  }
};
```

Dump of `CalleeExpr` is:

```
RecoveryExpr 0x106671e8 '' contains-errors lvalue
|-ParenExpr 0x10667020 'struct TestBadPMF' lvalue
| `-UnaryOperator 0x10667008 'struct TestBadPMF' lvalue prefix '*' cannot 
overflow
|   `-CXXThisExpr 0x10666ff8 'struct TestBadPMF *' this
`-MemberExpr 0x10667050 'int (struct TestBadPMF::*)(void)' lvalue ->pmf 
0x10666ed0
  `-CXXThisExpr 0x10667040 'struct TestBadPMF *' implicit this
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337576.
haberman marked 6 inline comments as done.
haberman added a comment.

- Word-smithed diagnostics and addressed other review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,263 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{tail call cannot invoke function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{tail call cannot invoke function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{tail call cannot invoke function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{tail call in non-member function cannot invoke non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{tail call in non-static member function cannot invoke non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXM

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337581.
haberman added a comment.

- More diagnostic wordsmithing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,269 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{cannot perform a tail call to function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{cannot perform a tail call to function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{cannot perform a tail call to function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{non-member function cannot perform a tail call to non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{non-static member function cannot perform a tail call to non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the diagnostic cannot currently distinguish betw

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337589.
haberman added a comment.

- Added release note for [[clang::musttail]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,269 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{cannot perform a tail call to function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{cannot perform a tail call to function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{cannot perform a tail call to function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{non-member function cannot perform a tail call to non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{non-static member function cannot perform a tail call to non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the 

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337590.
haberman added a comment.

- Fixed release note escaping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,269 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{cannot perform a tail call to function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{cannot perform a tail call to function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{cannot perform a tail call to function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{non-member function cannot perform a tail call to non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{non-static member function cannot perform a tail call to non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the diagnostic cann

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337592.
haberman added a comment.

- Fixed several cases in CodeGen test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,269 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{cannot perform a tail call to function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{cannot perform a tail call to function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{cannot perform a tail call to function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{non-member function cannot perform a tail call to non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{non-static member function cannot perform a tail call to non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the diagnos

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman updated this revision to Diff 337597.
haberman added a comment.

- Fixed typo in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-musttail.cpp
  clang/test/Sema/attr-musttail.c
  clang/test/Sema/attr-musttail.m
  clang/test/SemaCXX/attr-musttail.cpp

Index: clang/test/SemaCXX/attr-musttail.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-musttail.cpp
@@ -0,0 +1,269 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-extensions -fcxx-exceptions -fopenmp %s
+
+int ReturnsInt1();
+int Func1() {
+  [[clang::musttail]] ReturnsInt1();  // expected-error {{'musttail' attribute only applies to return statements}}
+  [[clang::musttail(1, 2)]] return ReturnsInt1(); // expected-error {{'musttail' attribute takes no arguments}}
+  [[clang::musttail]] return 5;   // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+  [[clang::musttail]] return ReturnsInt1();
+}
+
+void NoFunctionCall() {
+  [[clang::musttail]] return; // expected-error {{'musttail' attribute requires that the return value is the result of a function call}}
+}
+
+[[clang::musttail]] static int int_val = ReturnsInt1(); // expected-error {{'musttail' attribute cannot be applied to a declaration}}
+
+void NoParams(); // expected-note {{target function has different number of parameters (expected 1 but has 0)}}
+void TestParamArityMismatch(int x) {
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return NoParams();  // expected-error {{cannot perform a tail call to function 'NoParams' because its signature is incompatible with the calling function}}
+}
+
+void LongParam(long x); // expected-note {{target function has type mismatch at 1st parameter (expected 'long' but has 'int')}}
+void TestParamTypeMismatch(int x) {
+  [[clang::musttail]]  // expected-note {{tail call required by 'musttail' attribute here}}
+  return LongParam(x); // expected-error {{cannot perform a tail call to function 'LongParam' because its signature is incompatible with the calling function}}
+}
+
+long ReturnsLong(); // expected-note {{target function has different return type ('int' expected but has 'long')}}
+int TestReturnTypeMismatch() {
+  [[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+  return ReturnsLong(); // expected-error {{cannot perform a tail call to function 'ReturnsLong' because its signature is incompatible with the calling function}}
+}
+
+struct Struct1 {
+  void MemberFunction(); // expected-note {{'MemberFunction' declared here}}
+};
+void TestNonMemberToMember() {
+  Struct1 st;
+  [[clang::musttail]] // expected-note {{tail call required by 'musttail' attribute here}}
+  return st.MemberFunction(); // expected-error {{non-member function cannot perform a tail call to non-static member function 'MemberFunction'}}
+}
+
+void ReturnsVoid(); // expected-note {{'ReturnsVoid' declared here}}
+struct Struct2 {
+  void TestMemberToNonMember() {
+[[clang::musttail]]   // expected-note {{tail call required by 'musttail' attribute here}}
+return ReturnsVoid(); // expected-error{{non-static member function cannot perform a tail call to non-member function 'ReturnsVoid'}}
+  }
+};
+
+class HasNonTrivialDestructor {
+public:
+  ~HasNonTrivialDestructor() {}
+  int ReturnsInt();
+};
+
+void ReturnsVoid2();
+void TestNonTrivialDestructorInScope() {
+  HasNonTrivialDestructor foo;  // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  [[clang::musttail]] return ReturnsVoid(); // expected-error {{cannot perform a tail call from this return statement}}
+}
+
+int NonTrivialParam(HasNonTrivialDestructor x);
+int TestNonTrivialParam(HasNonTrivialDestructor x) {
+  [[clang::musttail]] return NonTrivialParam(x); // expected-error {{tail call requires that the return value, all parameters, and any temporaries created by the expression are trivially destructible}}
+}
+
+HasNonTrivialDestructor ReturnsNonTrivialValue();
+HasNonTrivialDestructor TestReturnsNonTrivialValue() {
+  // FIXME: the diagnostic cannot cur

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added a comment.

Ok I think this is ready to land.

There are a few FIXME comments, I will follow up with some small changes to 
address them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D99983: Provide TreeTransform::TransformAttr the transformed statement; NFC

[Josh Haberman via Phabricator via cfe-commits]

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

This seems fine to me, but I'll defer to @rsmith for final review.


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

https://reviews.llvm.org/D99983

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