From 25673aaedacdd2a345407e29155be5ce99bdbbd4 Mon Sep 17 00:00:00 2001
From: Stephan Bergmann <stephan.bergmann@allotropia.de>
Date: Tue, 5 Nov 2024 08:37:12 +0100
Subject: [PATCH] Fix implementation of loplugin::isDerivedFrom

...where clang::CXXRecordDecl::forallBases is documented as:  "This routine
returns false if the class has non-computable base classes."  So, presumably
since
<https://github.com/llvm/llvm-project/commit/bf099f4682bf088aaa49b2c72fb1ef3250213fbb>
"[llvm][ADT] Structured bindings for move-only types in `StringMap` (#114676)"
changed

> -template <std::size_t I, typename ValueTy>
> -struct tuple_element<I, llvm::StringMapEntry<ValueTy>>
> -    : std::conditional<I == 0, llvm::StringRef, ValueTy> {};
> +template <std::size_t Index, typename ValueTy>
> +struct std::tuple_element<Index, llvm::StringMapEntry<ValueTy>>
> +    : std::tuple_element<Index, std::pair<llvm::StringRef, ValueTy>> {};

in LLVM's llvm/include/llvm/ADT/StringMapEntry.h, our !forallBases check here
started to trivially always be true for that struct tuple_element
specialization, so the isDerivedFrom check in
CheckFileVisitor::VisitCXXRecordDecl in
compilerplugins/clang/sharedvisitor/analyzer.cxx started to erroneously be true
for that struct, so it started to generate
compilerplugins/clang/sharedvisitor/*.plugininfo files with bogus extra

> InfoVersion:1
> ClassName:tuple_element
> InfoEnd

content, which in turn caused the generated
compilerplugins/clang/sharedvisitor/sharedvisitor.cxx to contain lots of

> #include "tuple_element.cxx"

and other nonsense, which caused the build to break with

> [CXX] compilerplugins/clang/sharedvisitor/sharedvisitor.cxx
> lo/core/compilerplugins/clang/sharedvisitor/sharedvisitor.cxx:20:10: fatal error: 'tuple_element.cxx' file not found
>    20 | #include "tuple_element.cxx"
>       |          ^~~~~~~~~~~~~~~~~~~

etc.

(And now spelling out the implementation of forAnyBase here also reveals that
BaseCheckSubclass will never be called with a null BaseDefinition, so the code
handling that has been removed.)

Change-Id: I8a6e42260eae86852ec37a80d058777653fac394
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/176042
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
---
 compilerplugins/clang/check.cxx | 34 ++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/compilerplugins/clang/check.cxx b/compilerplugins/clang/check.cxx
index 60e476cc37b7..9fe576db612f 100644
--- a/compilerplugins/clang/check.cxx
+++ b/compilerplugins/clang/check.cxx
@@ -374,14 +374,34 @@ bool isOkToRemoveArithmeticCast(
 }
 
 
-static bool BaseCheckNotSubclass(const clang::CXXRecordDecl *BaseDefinition, void *p) {
-    if (!BaseDefinition)
-        return true;
+static bool BaseCheckSubclass(const clang::CXXRecordDecl *BaseDefinition, void *p) {
+    assert(BaseDefinition != nullptr);
     auto const & base = *static_cast<const DeclChecker *>(p);
     if (base(BaseDefinition)) {
-        return false;
+        return true;
     }
-    return true;
+    return false;
+}
+
+bool forAnyBase(
+    clang::CXXRecordDecl const * decl, clang::CXXRecordDecl::ForallBasesCallback matches)
+{
+    // Based on the implementation of clang::CXXRecordDecl::forallBases in LLVM's
+    // clang/lib/AST/CXXInheritance.cpp:
+    for (auto const & i: decl->bases()) {
+        auto const t = i.getType()->getAs<clang::RecordType>();
+        if (t == nullptr) {
+            return false;
+        }
+        auto const b = llvm::cast_or_null<clang::CXXRecordDecl>(t->getDecl()->getDefinition());
+        if (b == nullptr || (b->isDependentContext() && !b->isCurrentInstantiation(decl))) {
+            return false;
+        }
+        if (matches(b) || forAnyBase(b, matches)) {
+            return true;
+        }
+    }
+    return false;
 }
 
 bool isDerivedFrom(const clang::CXXRecordDecl *decl, DeclChecker base, bool checkSelf) {
@@ -392,9 +412,9 @@ bool isDerivedFrom(const clang::CXXRecordDecl *decl, DeclChecker base, bool chec
     if (!decl->hasDefinition()) {
         return false;
     }
-    if (!decl->forallBases(
+    if (forAnyBase(decl,
             [&base](const clang::CXXRecordDecl *BaseDefinition) -> bool
-                { return BaseCheckNotSubclass(BaseDefinition, &base); }))
+                { return BaseCheckSubclass(BaseDefinition, &base); }))
     {
         return true;
     }