compilerplugins/clang/check.cxx | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-)
New commits: commit 25673aaedacdd2a345407e29155be5ce99bdbbd4 Author: Stephan Bergmann <[email protected]> AuthorDate: Tue Nov 5 08:37:12 2024 +0100 Commit: Stephan Bergmann <[email protected]> CommitDate: Tue Nov 5 12:47:54 2024 +0100 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 <[email protected]> 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; }
