[clang] 6e73fee - List implicit operator== after implicit destructors in a vtable.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-01-21T15:54:40-08:00
New Revision: 6e73fee780839bfa95aff492864e93e79910380b

URL: https://github.com/llvm/llvm-project/commit/6e73fee780839bfa95aff492864e93e79910380b
DIFF: https://github.com/llvm/llvm-project/commit/6e73fee780839bfa95aff492864e93e79910380b.diff

LOG: List implicit operator== after implicit destructors in a vtable.

Summary:
We previously listed first declared members, then implicit operator=,
then implicit operator==, then implicit destructors. Per discussion on
https://github.com/itanium-cxx-abi/cxx-abi/issues/88, put the implicit
equality comparison operators at the very end, after all special member
functions.

This reinstates add2b7e44ada46f30715b5c48823a9e9e317e0c3, reverted in
commit 89e43f04ba87a0da6e94863db149669c7536486b, with a fix for 32-bit
targets.

Reviewers: rjmccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 
    clang/test/CodeGenCXX/virtual-compare.cpp

Modified: 
    clang/lib/AST/VTableBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp
index 2b5b74be5961..0bff976905fc 100644
--- a/clang/lib/AST/VTableBuilder.cpp
+++ b/clang/lib/AST/VTableBuilder.cpp
@@ -1474,11 +1474,11 @@ void ItaniumVTableBuilder::AddMethods(
       llvm_unreachable("Found a duplicate primary base!");
   }
 
-  const CXXDestructorDecl *ImplicitVirtualDtor = nullptr;
-
   typedef llvm::SmallVector<const CXXMethodDecl *, 8> NewVirtualFunctionsTy;
   NewVirtualFunctionsTy NewVirtualFunctions;
 
+  llvm::SmallVector<const CXXMethodDecl*, 4> NewImplicitVirtualFunctions;
+
   // Now go through all virtual member functions and add them.
   for (const auto *MD : RD->methods()) {
     if (!MD->isVirtual())
@@ -1542,24 +1542,30 @@ void ItaniumVTableBuilder::AddMethods(
       }
     }
 
-    if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
-      if (MD->isImplicit()) {
-        // Itanium C++ ABI 2.5.2:
-        //   If a class has an implicitly-defined virtual destructor,
-        //   its entries come after the declared virtual function pointers.
-
-        assert(!ImplicitVirtualDtor &&
-               "Did already see an implicit virtual dtor!");
-        ImplicitVirtualDtor = DD;
-        continue;
-      }
-    }
-
-    NewVirtualFunctions.push_back(MD);
-  }
-
-  if (ImplicitVirtualDtor)
-    NewVirtualFunctions.push_back(ImplicitVirtualDtor);
+    if (MD->isImplicit())
+      NewImplicitVirtualFunctions.push_back(MD);
+    else
+      NewVirtualFunctions.push_back(MD);
+  }
+
+  std::stable_sort(
+      NewImplicitVirtualFunctions.begin(), NewImplicitVirtualFunctions.end(),
+      [](const CXXMethodDecl *A, const CXXMethodDecl *B) {
+        if (A->isCopyAssignmentOperator() != B->isCopyAssignmentOperator())
+          return A->isCopyAssignmentOperator();
+        if (A->isMoveAssignmentOperator() != B->isMoveAssignmentOperator())
+          return A->isMoveAssignmentOperator();
+        if (isa<CXXDestructorDecl>(A) != isa<CXXDestructorDecl>(B))
+          return isa<CXXDestructorDecl>(A);
+        assert(A->getOverloadedOperator() == OO_EqualEqual &&
+               B->getOverloadedOperator() == OO_EqualEqual &&
+               "unexpected or duplicate implicit virtual function");
+        // We rely on Sema to have declared the operator== members in the
+        // same order as the corresponding operator<=> members.
+        return false;
+      });
+  NewVirtualFunctions.append(NewImplicitVirtualFunctions.begin(),
+                             NewImplicitVirtualFunctions.end());
 
   for (const CXXMethodDecl *MD : NewVirtualFunctions) {
     // Get the final overrider.

diff  --git a/clang/test/CodeGenCXX/virtual-compare.cpp b/clang/test/CodeGenCXX/virtual-compare.cpp
new file mode 100644
index 000000000000..ef75513ec0fe
--- /dev/null
+++ b/clang/test/CodeGenCXX/virtual-compare.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -std=c++2a -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/std-compare.h"
+
+// CHECK: @_ZTV1A =
+struct A;
+struct X {
+  // CHECK-SAME: @_ZN1X1xEv
+  virtual void x();
+  friend auto operator<=>(X, X) = default;
+};
+struct Y {
+  virtual ~Y();
+  virtual A &operator=(const A &);
+  friend auto operator<=>(Y, Y) = default;
+};
+struct A : X, Y {
+  // CHECK-SAME: @_ZN1A1fEv
+  virtual void f();
+  // CHECK-SAME: @_ZNKR1AssERKS_
+  virtual std::strong_ordering operator<=>(const A &) const & = default;
+  // CHECK-SAME: @_ZN1A1gEv
+  virtual void g();
+  // CHECK-SAME: @_ZNKO1AssERKS_
+  virtual std::strong_ordering operator<=>(const A &) const && = default;
+  // CHECK-SAME: @_ZN1A1hEv
+  virtual void h();
+
+  // CHECK-SAME: @_ZN1AaSERKS_
+  // implicit virtual A &operator=(const A&) = default;
+
+  // CHECK-SAME: @_ZN1AD1Ev
+  // CHECK-SAME: @_ZN1AD0Ev
+  // implicit virtual ~A();
+
+  // CHECK-SAME: @_ZNKR1AeqERKS_
+  // implicit virtual A &operator==(const A&) const & = default;
+
+  // CHECK-SAME: @_ZNKO1AeqERKS_
+  // implicit virtual A &operator==(const A&) const && = default;
+};
+
+// For Y:
+// CHECK-SAME: @_ZTI1A
+
+// CHECK-SAME: @_ZThn{{[0-9]*}}_N1AD1Ev
+// CHECK-SAME: @_ZThn{{[0-9]*}}_N1AD0Ev
+// virtual ~Y();
+
+// CHECK-SAME: @_ZThn{{[0-9]*}}_N1AaSERKS_
+// virtual A &operator=(const A &);
+
+void A::f() {}