[clang] [clang][Sema] Fix particular operator overload crash (PR #105976)

Nicolas van Kempen via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 2 21:46:34 PDT 2024


https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/105976

>From 465eadc6e5f83470764a14071f62f272c3cd7921 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankemp at gmail.com>
Date: Tue, 3 Sep 2024 00:43:30 -0400
Subject: [PATCH] [clang] Keep using declarations at head of list in
 addOrReplaceDecl

Fix #104883.
Fix #104800.

0cb7e7ca0c864e052bf49978f3bcd667c9e16930 is the culprit by bisection. The reason
for the break is that prior to this commit, using declarations were always kept
at the front of the list. This behavior was changed in that commit.

Here, we re-establish the behavior of keeping using declarations at the head in
`addOrReplaceDecl`.
---
 .../include/clang/AST/DeclContextInternals.h  | 56 +++++++++++++++----
 clang/test/SemaCXX/PR105976.cpp               | 52 +++++++++++++++++
 2 files changed, 96 insertions(+), 12 deletions(-)
 create mode 100644 clang/test/SemaCXX/PR105976.cpp

diff --git a/clang/include/clang/AST/DeclContextInternals.h b/clang/include/clang/AST/DeclContextInternals.h
index e169c485921929..391cf6a2b5bfd3 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -234,6 +234,15 @@ class StoredDeclsList {
         return;
       }
 
+      // Keep using declarations at the front of the list.
+      if (D->getIdentifierNamespace() & Decl::IDNS_Using) {
+        ASTContext &C = D->getASTContext();
+        DeclListNode *Node = C.AllocateDeclListNode(D);
+        Node->Rest = Data.getPointer();
+        Data.setPointer(Node);
+        return;
+      }
+
       // Add D after OldD.
       ASTContext &C = D->getASTContext();
       DeclListNode *Node = C.AllocateDeclListNode(OldD);
@@ -245,27 +254,50 @@ class StoredDeclsList {
     // FIXME: Move the assert before the single decl case when we fix the
     // duplication coming from the ASTReader reading builtin types.
     assert(!llvm::is_contained(getLookupResult(), D) && "Already exists!");
-    // Determine if this declaration is actually a redeclaration.
-    for (DeclListNode *N = getAsList(); /*return in loop*/;
-         N = N->Rest.dyn_cast<DeclListNode *>()) {
+
+    DeclListNode *Prev = nullptr;
+    for (DeclListNode *N = getAsList(); N;
+         Prev = N, N = N->Rest.dyn_cast<DeclListNode *>()) {
       if (D->declarationReplaces(N->D, IsKnownNewer)) {
         N->D = D;
         return;
       }
-      if (auto *ND = N->Rest.dyn_cast<NamedDecl *>()) {
-        if (D->declarationReplaces(ND, IsKnownNewer)) {
-          N->Rest = D;
-          return;
-        }
 
-        // Add D after ND.
+      // Keep using declarations at the front of the list.
+      if ((D->getIdentifierNamespace() & Decl::IDNS_Using) &&
+          N->D->getIdentifierNamespace() != Decl::IDNS_Using) {
         ASTContext &C = D->getASTContext();
-        DeclListNode *Node = C.AllocateDeclListNode(ND);
-        N->Rest = Node;
-        Node->Rest = D;
+        DeclListNode *Node = C.AllocateDeclListNode(D);
+        Node->Rest = N;
+        if (Prev)
+          Prev->Rest = Node;
+        else
+          Data.setPointer(Node);
         return;
       }
     }
+
+    // P->Rest must be the tail NamedDecl* at this point.
+    NamedDecl *N = Prev->Rest.get<NamedDecl *>();
+    if (D->declarationReplaces(N, IsKnownNewer)) {
+      Prev->Rest = D;
+      return;
+    }
+
+    // Keep using declarations at the front of the list.
+    if (D->getIdentifierNamespace() & Decl::IDNS_Using) {
+      ASTContext &C = D->getASTContext();
+      DeclListNode *Node = C.AllocateDeclListNode(D);
+      Node->Rest = Data.getPointer();
+      Data.setPointer(Node);
+      return;
+    }
+
+    // Add D after N.
+    ASTContext &C = D->getASTContext();
+    DeclListNode *Node = C.AllocateDeclListNode(N);
+    Prev->Rest = Node;
+    Node->Rest = D;
   }
 
   /// Add a declaration to the list without checking if it replaces anything.
diff --git a/clang/test/SemaCXX/PR105976.cpp b/clang/test/SemaCXX/PR105976.cpp
new file mode 100644
index 00000000000000..238b386b7270bd
--- /dev/null
+++ b/clang/test/SemaCXX/PR105976.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -verify-ignore-unexpected=note %s
+
+struct A1 {
+  void operator==(A1);
+};
+
+struct A2 {
+  void operator==(A2);
+};
+
+struct A3 {
+  void operator==(A3);
+};
+
+struct A4 : A1, A2 {
+  using A1::operator==;
+  using A2::operator==;
+};
+
+struct A5 : A3, A4 {};
+
+void A6() {
+  A5{} == A5{};
+  // expected-error at -1 {{member 'operator==' found in multiple base classes of different types}}
+  // expected-error at -2 {{use of overloaded operator '==' is ambiguous}}
+}
+
+
+struct B1 {
+  template <typename T> void operator==(T);
+};
+
+struct B2 {
+  template <typename T> void operator==(T);
+};
+
+struct B3 {
+  template <typename T> void operator==(T);
+};
+
+struct B4 : B1, B2 {
+  using B1::operator==;
+  using B2::operator==;
+};
+
+struct B5 : B3, B4 {};
+
+void B6() {
+  B5{} == B5{};
+  // expected-error at -1 {{member 'operator==' found in multiple base classes of different types}}
+  // expected-error at -2 {{use of overloaded operator '==' is ambiguous}}
+}



More information about the cfe-commits mailing list