[clang] 4500f0a - [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2021-02-25T14:58:18-08:00
New Revision: 4500f0a7321d75a061d09d92987b522e9cc09517

URL: https://github.com/llvm/llvm-project/commit/4500f0a7321d75a061d09d92987b522e9cc09517
DIFF: https://github.com/llvm/llvm-project/commit/4500f0a7321d75a061d09d92987b522e9cc09517.diff

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

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.

Differential Revision: https://reviews.llvm.org/D96832

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index d7d74bd6eee7..b88b18d37477 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4057,9 +4057,8 @@ def NotTailCalledDocs : Documentation {
   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:
 
@@ -4085,28 +4084,25 @@ However, it doesn't prevent tail-call optimization in this case:
       return (*fn)(a);
     }
 
-Marking virtual functions as ``not_tail_called`` is an error:
+Generally, marking an overriding virtual function as ``not_tail_called`` is
+not useful, because this attribute is a property of the static type. Calls
+made through a pointer or reference to the base class type will respect
+the ``not_tail_called`` attribute of the base class's member function,
+regardless of the runtime destination of the call:
 
   .. 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;
+    struct Foo { virtual void f(); };
+    struct Bar : Foo {
+      [[clang::not_tail_called]] void f() override;
     };
+    void callera(Bar& bar) {
+      Foo& foo = bar;
+      // not_tail_called has no effect on here, even though the
+      // underlying method is f from Bar.
+      foo.f();
+      bar.f(); // No tail-call optimization on here.
+    }
   }];
 }
 

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e1e14ae631af..50a29e29eb54 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6432,16 +6432,6 @@ static void checkAttributesAfterMerging(Sema &S, NamedDecl &ND) {
     }
   }
 
-  // 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;

diff  --git a/clang/test/CodeGenCXX/attr-notail.cpp b/clang/test/CodeGenCXX/attr-notail.cpp
index ecc9b55ccdd9..5e207cd3a280 100644
--- a/clang/test/CodeGenCXX/attr-notail.cpp
+++ b/clang/test/CodeGenCXX/attr-notail.cpp
@@ -4,14 +4,28 @@ class Class1 {
 public:
   [[clang::not_tail_called]] int m1();
   int m2();
+  [[clang::not_tail_called]] virtual int m3();
+  virtual int m4();
 };
 
-int foo1(int a, Class1 *c1) {
+class Class2: public Class1 {
+public:
+  [[clang::not_tail_called]] int m4() override;
+};
+
+int foo1(int a, Class1 *c1, Class2 &c2) {
   if (a)
     return c1->m1();
+  c1->m3();
+  Class1 &c = c2;
+  c.m4();
+  c2.m4();
   return c1->m2();
 }
 
-// CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1(
+// CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1R6Class2(
 // CHECK: %{{[a-z0-9]+}} = notail call i32 @_ZN6Class12m1Ev(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1*
+// CHECK-NOT: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class2* 
 // CHECK: %{{[a-z0-9]+}} = call i32 @_ZN6Class12m2Ev(%class.Class1*

diff  --git a/clang/test/SemaCXX/attr-notail.cpp b/clang/test/SemaCXX/attr-notail.cpp
index 2f39746dc85b..2303a3cc54b7 100644
--- a/clang/test/SemaCXX/attr-notail.cpp
+++ b/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 Base {
 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();
 };