[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Zequan Wu via Phabricator via cfe-commits]

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

It would be beneficial to allow not_tail_called attribute to be applied to
virtual functions. I don't see any drawback of allowing this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96832

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/attr-notail.cpp
  clang/test/SemaCXX/attr-notail.cpp


Index: clang/test/SemaCXX/attr-notail.cpp
===================================================================
--- clang/test/SemaCXX/attr-notail.cpp
+++ clang/test/SemaCXX/attr-notail.cpp
@@ -1,8 +1,9 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 class Base {
 public:
-  [[clang::not_tail_called]] virtual int foo1(); // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] virtual int foo1();
   virtual int foo2();
   [[clang::not_tail_called]] int foo3();
   virtual ~Base() {}
@@ -11,6 +12,6 @@
 class Derived1 : public Base {
 public:
   int foo1() override;
-  [[clang::not_tail_called]] int foo2() override; // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] int foo2() override;
   [[clang::not_tail_called]] int foo4();
 };
Index: clang/test/CodeGenCXX/attr-notail.cpp
===================================================================
--- clang/test/CodeGenCXX/attr-notail.cpp
+++ clang/test/CodeGenCXX/attr-notail.cpp
@@ -4,14 +4,17 @@
 public:
   [[clang::not_tail_called]] int m1();
   int m2();
+  [[clang::not_tail_called]] virtual int m3();
 };
 
 int foo1(int a, Class1 *c1) {
   if (a)
     return c1->m1();
+  c1->m3();
   return c1->m2();
 }
 
 // CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1(
 // CHECK: %{{[a-z0-9]+}} = notail call i32 @_ZN6Class12m1Ev(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1* 
 // CHECK: %{{[a-z0-9]+}} = call i32 @_ZN6Class12m2Ev(%class.Class1*
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6432,16 +6432,6 @@
     }
   }
 
-  // Virtual functions cannot be marked as 'notail'.
-  if (auto *Attr = ND.getAttr<NotTailCalledAttr>())
-    if (auto *MD = dyn_cast<CXXMethodDecl>(&ND))
-      if (MD->isVirtual()) {
-        S.Diag(ND.getLocation(),
-               diag::err_invalid_attribute_on_virtual_function)
-            << Attr;
-        ND.dropAttr<NotTailCalledAttr>();
-      }
-
   // Check the attributes on the function type, if any.
   if (const auto *FD = dyn_cast<FunctionDecl>(&ND)) {
     // Don't declare this variable in the second operand of the for-statement;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4057,9 +4057,8 @@
   let Category = DocCatFunction;
   let Content = [{
 The ``not_tail_called`` attribute prevents tail-call optimization on statically
-bound calls. It has no effect on indirect calls. Virtual functions, objective-c
-methods, and functions marked as ``always_inline`` cannot be marked as
-``not_tail_called``.
+bound calls. Objective-c methods, and functions marked as ``always_inline`` 
+cannot be marked as ``not_tail_called``.
 
 For example, it prevents tail-call optimization in the following case:
 
@@ -4070,43 +4069,6 @@
     int foo2(int a) {
       return foo1(a); // No tail-call optimization on direct calls.
     }
-
-However, it doesn't prevent tail-call optimization in this case:
-
-  .. code-block:: c
-
-    int __attribute__((not_tail_called)) foo1(int);
-
-    int foo2(int a) {
-      int (*fn)(int) = &foo1;
-
-      // not_tail_called has no effect on an indirect call even if the call can
-      // be resolved at compile time.
-      return (*fn)(a);
-    }
-
-Marking virtual functions as ``not_tail_called`` is an error:
-
-  .. code-block:: c++
-
-    class Base {
-    public:
-      // not_tail_called on a virtual function is an error.
-      [[clang::not_tail_called]] virtual int foo1();
-
-      virtual int foo2();
-
-      // Non-virtual functions can be marked ``not_tail_called``.
-      [[clang::not_tail_called]] int foo3();
-    };
-
-    class Derived1 : public Base {
-    public:
-      int foo1() override;
-
-      // not_tail_called on a virtual function is an error.
-      [[clang::not_tail_called]] int foo2() override;
-    };
   }];
 }
 

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