From 913d34ec6b8fdb2796f76ce90fee51ade2051189 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Tue, 12 Nov 2019 21:53:31 +0100 Subject: Extend loplugin:salbool to loplugin:fakebool ...checking for unnecessary uses of more "fake bool" types. In the past, some of the checks involving the types of variables or data members, or the return types of functions, issued warnings that required surrounding code to be changed too (e.g., when changing the signature of a function whose address was taken). These checks have been tightened now to not warn in such cases (which avoids warnings that require changes to additional code, or changes that might even be impossible to make, at the cost of being less aggressive about removing all unnecessary uses of those "fake bool" types). Change-Id: I70eb75039817cda34ed611387ee27dc5f36a3e2e Reviewed-on: https://gerrit.libreoffice.org/82554 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- compilerplugins/clang/fakebool.cxx | 1056 +++++++++++++++++++++++++++++++ compilerplugins/clang/salbool.cxx | 832 ------------------------ compilerplugins/clang/test/fakebool.cxx | 22 + compilerplugins/clang/test/salbool.cxx | 18 - 4 files changed, 1078 insertions(+), 850 deletions(-) create mode 100644 compilerplugins/clang/fakebool.cxx delete mode 100644 compilerplugins/clang/salbool.cxx create mode 100644 compilerplugins/clang/test/fakebool.cxx delete mode 100644 compilerplugins/clang/test/salbool.cxx (limited to 'compilerplugins') diff --git a/compilerplugins/clang/fakebool.cxx b/compilerplugins/clang/fakebool.cxx new file mode 100644 index 000000000000..1dbb535ceef9 --- /dev/null +++ b/compilerplugins/clang/fakebool.cxx @@ -0,0 +1,1056 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include +#include +#include +#include +#include + +#include "clang/AST/Attr.h" + +#include "check.hxx" +#include "compat.hxx" +#include "functionaddress.hxx" +#include "plugin.hxx" + +namespace { + +// BEGIN code copied from LLVM's clang/lib/Sema/Sema.cpp + +typedef llvm::DenseMap RecordCompleteMap; + +/// Returns true, if all methods and nested classes of the given +/// CXXRecordDecl are defined in this translation unit. +/// +/// Should only be called from ActOnEndOfTranslationUnit so that all +/// definitions are actually read. +static bool MethodsAndNestedClassesComplete(const CXXRecordDecl *RD, + RecordCompleteMap &MNCComplete) { + RecordCompleteMap::iterator Cache = MNCComplete.find(RD); + if (Cache != MNCComplete.end()) + return Cache->second; + if (!RD->isCompleteDefinition()) + return false; + bool Complete = true; + for (DeclContext::decl_iterator I = RD->decls_begin(), + E = RD->decls_end(); + I != E && Complete; ++I) { + if (const CXXMethodDecl *M = dyn_cast(*I)) + Complete = M->isDefined() || M->isDefaulted() || + (M->isPure() && !isa(M)); + else if (const FunctionTemplateDecl *F = dyn_cast(*I)) + // If the template function is marked as late template parsed at this + // point, it has not been instantiated and therefore we have not + // performed semantic analysis on it yet, so we cannot know if the type + // can be considered complete. + Complete = !F->getTemplatedDecl()->isLateTemplateParsed() && + F->getTemplatedDecl()->isDefined(); + else if (const CXXRecordDecl *R = dyn_cast(*I)) { + if (R->isInjectedClassName()) + continue; + if (R->hasDefinition()) + Complete = MethodsAndNestedClassesComplete(R->getDefinition(), + MNCComplete); + else + Complete = false; + } + } + MNCComplete[RD] = Complete; + return Complete; +} + +/// Returns true, if the given CXXRecordDecl is fully defined in this +/// translation unit, i.e. all methods are defined or pure virtual and all +/// friends, friend functions and nested classes are fully defined in this +/// translation unit. +/// +/// Should only be called from ActOnEndOfTranslationUnit so that all +/// definitions are actually read. +static bool IsRecordFullyDefined(const CXXRecordDecl *RD, + RecordCompleteMap &RecordsComplete, + RecordCompleteMap &MNCComplete) { + RecordCompleteMap::iterator Cache = RecordsComplete.find(RD); + if (Cache != RecordsComplete.end()) + return Cache->second; + bool Complete = MethodsAndNestedClassesComplete(RD, MNCComplete); + for (CXXRecordDecl::friend_iterator I = RD->friend_begin(), + E = RD->friend_end(); + I != E && Complete; ++I) { + // Check if friend classes and methods are complete. + if (TypeSourceInfo *TSI = (*I)->getFriendType()) { + // Friend classes are available as the TypeSourceInfo of the FriendDecl. + if (CXXRecordDecl *FriendD = TSI->getType()->getAsCXXRecordDecl()) + Complete = MethodsAndNestedClassesComplete(FriendD, MNCComplete); + else + Complete = false; + } else { + // Friend functions are available through the NamedDecl of FriendDecl. + if (const FunctionDecl *FD = + dyn_cast((*I)->getFriendDecl())) + Complete = FD->isDefined(); + else + // This is a template friend, give up. + Complete = false; + } + } + RecordsComplete[RD] = Complete; + return Complete; +} + +RecordCompleteMap RecordsComplete; +RecordCompleteMap MNCComplete; + +// END code copied from LLVM's clang/lib/Sema/Sema.cpp + +// Is all code that could see `decl` defined in this TU? +bool isAllRelevantCodeDefined(NamedDecl const * decl) { + switch (decl->getAccess()) { + case AS_protected: + if (!cast(decl->getDeclContext())->hasAttr()) { + break; + } + LLVM_FALLTHROUGH; + case AS_private: + if (IsRecordFullyDefined( + cast(decl->getDeclContext()), RecordsComplete, MNCComplete)) + { + return true; + } + break; + default: + break; + } + return !decl->isExternallyVisible(); +} + +enum FakeBoolKind { + FBK_No, + FBK_BOOL, FBK_First = FBK_BOOL, + FBK_Boolean, FBK_FT_Bool, FBK_FcBool, FBK_GLboolean, FBK_NPBool, FBK_TW_BOOL, FBK_UBool, + FBK_boolean, FBK_dbus_bool_t, FBK_gboolean, FBK_hb_boot_t, FBK_jboolean, FBK_my_bool, + FBK_sal_Bool, + FBK_End }; + // matches loplugin::TypeCheck::AnyBoolean (compilerplugins/clang/check.hxx) + +StringRef getName(FakeBoolKind k) { + static constexpr llvm::StringLiteral names[] = { + "BOOL", "Boolean", "FT_Bool", "FcBool", "GLboolean", "NPBool", "TW_BOOL", "UBool", + "boolean", "dbus_bool_t", "gboolean", "hb_boot_t", "jboolean", "my_bool", "sal_Bool"}; + assert(k >= FBK_First && k < FBK_End); + return names[k - FBK_First]; +} + +FakeBoolKind isFakeBool(QualType type) { + TypedefType const * t = type->getAs(); + if (t != nullptr) { + auto const name = t->getDecl()->getName(); + for (int i = FBK_First; i != FBK_End; ++i) { + auto const k = FakeBoolKind(i); + if (name == getName(k)) { + return k; + } + } + } + return FBK_No; +} + +FakeBoolKind isFakeBoolArray(QualType type) { + auto t = type->getAsArrayTypeUnsafe(); + if (t == nullptr) { + return FBK_No; + } + auto const k = isFakeBool(t->getElementType()); + if (k != FBK_No) { + return k; + } + return isFakeBoolArray(t->getElementType()); +} + +// It appears that, given a function declaration, there is no way to determine +// the language linkage of the function's type, only of the function's name +// (via FunctionDecl::isExternC); however, in a case like +// +// extern "C" { static void f(); } +// +// the function's name does not have C language linkage while the function's +// type does (as clarified in C++11 [decl.link]); cf. +// "Language linkage of function type": +bool hasCLanguageLinkageType(FunctionDecl const * decl) { + assert(decl != nullptr); + if (decl->isExternC()) { + return true; + } + if (decl->isInExternCContext()) { + return true; + } + return false; +} + +enum class OverrideKind { NO, YES, MAYBE }; + +OverrideKind getOverrideKind(FunctionDecl const * decl) { + CXXMethodDecl const * m = dyn_cast(decl); + if (m == nullptr) { + return OverrideKind::NO; + } + if (m->size_overridden_methods() != 0 || m->hasAttr()) { + return OverrideKind::YES; + } + if (!dyn_cast(m->getDeclContext())->hasAnyDependentBases()) { + return OverrideKind::NO; + } + return OverrideKind::MAYBE; +} + +enum class BoolOverloadKind { No, Yes, CheckNext }; + +BoolOverloadKind isBoolOverloadOf( + FunctionDecl const * f, FunctionDecl const * decl, bool mustBeDeleted) +{ + if (!mustBeDeleted || f->isDeleted()) { + unsigned n = decl->getNumParams(); + if (f->getNumParams() == n) { + bool hasFB = false; + for (unsigned i = 0; i != n; ++i) { + QualType t1 { decl->getParamDecl(i)->getType() }; + bool isFB = isFakeBool(t1) != FBK_No; + bool isFBRef = !isFB && t1->isReferenceType() + && isFakeBool(t1.getNonReferenceType()) != FBK_No; + QualType t2 { f->getParamDecl(i)->getType() }; + if (!(isFB + ? t2->isBooleanType() + : isFBRef + ? (t2->isReferenceType() + && t2.getNonReferenceType()->isBooleanType()) + : t2.getCanonicalType() == t1.getCanonicalType())) + { + return BoolOverloadKind::CheckNext; + } + hasFB |= isFB || isFBRef; + } + return hasFB ? BoolOverloadKind::Yes : BoolOverloadKind::No; + // cheaply protect against the case where decl would have no + // fake bool parameters at all and would match itself + } + } + return BoolOverloadKind::CheckNext; +} + +//TODO: current implementation is not at all general, just tests what we +// encounter in practice: +bool hasBoolOverload(FunctionDecl const * decl, bool mustBeDeleted) { + auto ctx = decl->getDeclContext(); + if (!ctx->isLookupContext()) { + return false; + } + auto res = ctx->lookup(decl->getDeclName()); + for (auto d = res.begin(); d != res.end(); ++d) { + if (auto f = dyn_cast(*d)) { + switch (isBoolOverloadOf(f, decl, mustBeDeleted)) { + case BoolOverloadKind::No: + return false; + case BoolOverloadKind::Yes: + return true; + case BoolOverloadKind::CheckNext: + break; + } + } else if (auto ftd = dyn_cast(*d)) { + for (auto f: ftd->specializations()) { + if (f->getTemplateSpecializationKind() + == TSK_ExplicitSpecialization) + { + switch (isBoolOverloadOf(f, decl, mustBeDeleted)) { + case BoolOverloadKind::No: + return false; + case BoolOverloadKind::Yes: + return true; + case BoolOverloadKind::CheckNext: + break; + } + } + } + } + } + return false; +} + +class FakeBool: + public loplugin::FunctionAddress> +{ +public: + explicit FakeBool(loplugin::InstantiationData const & data): + FunctionAddress(data) {} + + virtual void run() override; + + bool VisitUnaryAddrOf(UnaryOperator const * op); + + bool VisitCallExpr(CallExpr * expr); + + bool VisitCStyleCastExpr(CStyleCastExpr * expr); + + bool VisitCXXStaticCastExpr(CXXStaticCastExpr * expr); + + bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr); + + bool VisitImplicitCastExpr(ImplicitCastExpr * expr); + + bool VisitReturnStmt(ReturnStmt const * stmt); + + bool WalkUpFromParmVarDecl(ParmVarDecl const * decl); + bool VisitParmVarDecl(ParmVarDecl const * decl); + + bool WalkUpFromVarDecl(VarDecl const * decl); + bool VisitVarDecl(VarDecl const * decl); + + bool WalkUpFromFieldDecl(FieldDecl const * decl); + bool VisitFieldDecl(FieldDecl const * decl); + + bool WalkUpFromFunctionDecl(FunctionDecl const * decl); + bool VisitFunctionDecl(FunctionDecl const * decl); + + bool VisitValueDecl(ValueDecl const * decl); + + bool TraverseStaticAssertDecl(StaticAssertDecl * decl); + + bool TraverseLinkageSpecDecl(LinkageSpecDecl * decl); + +private: + bool isFromCIncludeFile(SourceLocation spellingLocation) const; + + bool isSharedCAndCppCode(SourceLocation location) const; + + bool isInSpecialMainFile(SourceLocation spellingLocation) const; + + bool rewrite(SourceLocation location, FakeBoolKind kind); + + std::map varDecls_; + std::map fieldDecls_; + std::map parmVarDecls_; + std::map functionDecls_; + unsigned int externCContexts_ = 0; +}; + +void FakeBool::run() { + if (compiler.getLangOpts().CPlusPlus) { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + for (auto const dcl: varDecls_) { + auto const decl = dcl.first; auto const fbk = dcl.second; + SourceLocation loc { compat::getBeginLoc(decl) }; + TypeSourceInfo * tsi = decl->getTypeSourceInfo(); + if (tsi != nullptr) { + SourceLocation l { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getBeginLoc()) }; + SourceLocation end { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getEndLoc()) }; + assert(l.isFileID() && end.isFileID()); + if (l == end + || compiler.getSourceManager().isBeforeInTranslationUnit( + l, end)) + { + for (;;) { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), + compiler.getLangOpts()); + std::string s { + compiler.getSourceManager().getCharacterData(l), + n }; + if (s == getName(fbk)) { + loc = l; + break; + } + if (l == end) { + break; + } + l = l.getLocWithOffset(std::max(n, 1)); + } + } + } + if (!rewrite(loc, fbk)) { + report( + DiagnosticsEngine::Warning, + "VarDecl, use \"bool\" instead of %0", loc) + << decl->getType().getLocalUnqualifiedType() + << decl->getSourceRange(); + } + } + for (auto const dcl: fieldDecls_) { + auto const decl = dcl.first; auto const fbk = dcl.second; + SourceLocation loc { compat::getBeginLoc(decl) }; + TypeSourceInfo * tsi = decl->getTypeSourceInfo(); + if (tsi != nullptr) { + SourceLocation l { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getBeginLoc()) }; + SourceLocation end { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getEndLoc()) }; + assert(l.isFileID() && end.isFileID()); + if (l == end + || compiler.getSourceManager().isBeforeInTranslationUnit( + l, end)) + { + for (;;) { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), + compiler.getLangOpts()); + std::string s { + compiler.getSourceManager().getCharacterData(l), + n }; + if (s == getName(fbk)) { + loc = l; + break; + } + if (l == end) { + break; + } + l = l.getLocWithOffset(std::max(n, 1)); + } + } + } + if (!rewrite(loc, fbk)) { + report( + DiagnosticsEngine::Warning, + "FieldDecl, use \"bool\" instead of %0", loc) + << decl->getType().getLocalUnqualifiedType() << decl->getSourceRange(); + } + } + auto const ignoredFns = getFunctionsWithAddressTaken(); + for (auto const dcl: parmVarDecls_) { + auto const decl = dcl.first; auto const fbk = dcl.second; + FunctionDecl const * f = cast(decl->getDeclContext())->getCanonicalDecl(); + if (ignoredFns.find(f) != ignoredFns.end()) { + continue; + } + SourceLocation loc { compat::getBeginLoc(decl) }; + TypeSourceInfo * tsi = decl->getTypeSourceInfo(); + if (tsi != nullptr) { + SourceLocation l { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getBeginLoc()) }; + SourceLocation end { + compiler.getSourceManager().getExpansionLoc( + tsi->getTypeLoc().getEndLoc()) }; + assert(l.isFileID() && end.isFileID()); + if (l == end + || (compiler.getSourceManager() + .isBeforeInTranslationUnit(l, end))) + { + for (;;) { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), + compiler.getLangOpts()); + std::string s { + compiler.getSourceManager().getCharacterData(l), + n }; + if (s == getName(fbk)) { + loc = l; + break; + } + if (l == end) { + break; + } + l = l.getLocWithOffset(std::max(n, 1)); + } + } + } + // Only rewrite declarations in include files if a + // definition is also seen, to avoid compilation of a + // definition (in a main file only processed later) to fail + // with a "mismatch" error before the rewriter had a chance + // to act upon the definition (but use the heuristic of + // assuming pure virtual functions do not have definitions); + // also, do not automatically rewrite functions that could + // implicitly override depend base functions (and thus stop + // doing so after the rewrite; note that this is less + // dangerous for return types than for parameter types, + // where the function would still implicitly override and + // cause a compilation error due to the incompatible return + // type): + OverrideKind k = getOverrideKind(f); + if (!((compiler.getSourceManager().isInMainFile( + compiler.getSourceManager().getSpellingLoc( + dyn_cast( + decl->getDeclContext()) + ->getNameInfo().getLoc())) + || f->isDefined() || f->isPure()) + && k != OverrideKind::MAYBE && rewrite(loc, fbk))) + { + report( + DiagnosticsEngine::Warning, + ("ParmVarDecl, use \"bool\" instead of" + " %0%1"), + loc) + << decl->getType().getNonReferenceType().getLocalUnqualifiedType() + << (k == OverrideKind::MAYBE + ? (" (unless this member function overrides a" + " dependent base member function, even" + " though it is not marked 'override')") + : "") + << decl->getSourceRange(); + } + } + for (auto const dcl: functionDecls_) { + auto const decl = dcl.first; auto const fbk = dcl.second; + FunctionDecl const * f = decl->getCanonicalDecl(); + if (ignoredFns.find(f) != ignoredFns.end()) { + continue; + } + SourceLocation loc { compat::getBeginLoc(decl) }; + SourceLocation l { compiler.getSourceManager().getExpansionLoc( + loc) }; + SourceLocation end { compiler.getSourceManager().getExpansionLoc( + decl->getNameInfo().getLoc()) }; + assert(l.isFileID() && end.isFileID()); + if (compiler.getSourceManager().isBeforeInTranslationUnit(l, end)) { + while (l != end) { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), compiler.getLangOpts()); + std::string s { + compiler.getSourceManager().getCharacterData(l), n }; + if (s == getName(fbk)) { + loc = l; + break; + } + l = l.getLocWithOffset(std::max(n, 1)); + } + } + // Only rewrite declarations in include files if a definition is + // also seen, to avoid compilation of a definition (in a main file + // only processed later) to fail with a "mismatch" error before the + // rewriter had a chance to act upon the definition (but use the + // heuristic of assuming pure virtual functions do not have + // definitions): + if (!((compiler.getSourceManager().isInMainFile( + compiler.getSourceManager().getSpellingLoc( + decl->getNameInfo().getLoc())) + || f->isDefined() || f->isPure()) + && rewrite(loc, fbk))) + { + report( + DiagnosticsEngine::Warning, + "use \"bool\" instead of %0 as return type%1", + loc) + << decl->getReturnType().getNonReferenceType().getLocalUnqualifiedType() + << (getOverrideKind(f) == OverrideKind::MAYBE + ? (" (unless this member function overrides a dependent" + " base member function, even though it is not marked" + " 'override')") + : "") + << decl->getSourceRange(); + } + } + } +} + +bool FakeBool::VisitUnaryAddrOf(UnaryOperator const * op) { + FunctionAddress::VisitUnaryAddrOf(op); + Expr const * e1 = op->getSubExpr()->IgnoreParenCasts(); + if (isFakeBool(e1->getType()) != FBK_No) { + if (DeclRefExpr const * e2 = dyn_cast(e1)) { + VarDecl const * d = dyn_cast(e2->getDecl()); + if (d != nullptr) { + varDecls_.erase(d); + } + } else if (auto const e3 = dyn_cast(e1)) { + if (auto const d = dyn_cast(e3->getMemberDecl())) { + fieldDecls_.erase(d); + } + } + } + return true; +} + +bool FakeBool::VisitCallExpr(CallExpr * expr) { + Decl const * d = expr->getCalleeDecl(); + FunctionProtoType const * ft = nullptr; + if (d != nullptr) { + FunctionDecl const * fd = dyn_cast(d); + if (fd != nullptr) { + if (!hasBoolOverload(fd, false)) { + clang::PointerType const * pt = fd->getType() + ->getAs(); + QualType t2( + pt == nullptr ? fd->getType() : pt->getPointeeType()); + ft = t2->getAs(); + assert( + ft != nullptr || !compiler.getLangOpts().CPlusPlus + || (fd->getBuiltinID() != Builtin::NotBuiltin + && isa(t2))); + // __builtin_*s have no proto type? + } + } else { + VarDecl const * vd = dyn_cast(d); + if (vd != nullptr) { + clang::PointerType const * pt = vd->getType() + ->getAs(); + ft = (pt == nullptr ? vd->getType() : pt->getPointeeType()) + ->getAs(); + } + } + } + if (ft != nullptr) { + for (unsigned i = 0; i != ft->getNumParams(); ++i) { + QualType t(ft->getParamType(i)); + bool b = false; + if (t->isLValueReferenceType()) { + t = t.getNonReferenceType(); + b = !t.isConstQualified() && isFakeBool(t) != FBK_No; + } else if (t->isPointerType()) { + for (;;) { + auto t2 = t->getAs(); + if (t2 == nullptr) { + break; + } + t = t2->getPointeeType(); + } + b = isFakeBool(t) != FBK_No; + } + if (b && i < expr->getNumArgs()) { + auto const e1 = expr->getArg(i)->IgnoreParenImpCasts(); + if (DeclRefExpr * ref = dyn_cast(e1)) { + VarDecl const * d = dyn_cast(ref->getDecl()); + if (d != nullptr) { + varDecls_.erase(d); + } + } else if (auto const e2 = dyn_cast(e1)) { + if (auto const d = dyn_cast(e2->getMemberDecl())) { + fieldDecls_.erase(d); + } + } + } + } + } + return true; +} + +bool FakeBool::VisitCStyleCastExpr(CStyleCastExpr * expr) { + if (ignoreLocation(expr)) { + return true; + } + auto const k = isFakeBool(expr->getType()); + if (k != FBK_No) { + SourceLocation loc { compat::getBeginLoc(expr) }; + while (compiler.getSourceManager().isMacroArgExpansion(loc)) { + loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc); + } + if (compiler.getSourceManager().isMacroBodyExpansion(loc)) { + StringRef name { Lexer::getImmediateMacroName( + loc, compiler.getSourceManager(), compiler.getLangOpts()) }; + if (k == FBK_sal_Bool && (name == "sal_False" || name == "sal_True")) { + auto callLoc = compiler.getSourceManager() + .getImmediateMacroCallerLoc(loc); + if (!isSharedCAndCppCode(callLoc)) { + SourceLocation argLoc; + if (compiler.getSourceManager().isMacroArgExpansion( + compat::getBeginLoc(expr), &argLoc) + //TODO: check it's the complete (first) arg to the macro + && (Lexer::getImmediateMacroName( + argLoc, compiler.getSourceManager(), + compiler.getLangOpts()) + == "CPPUNIT_ASSERT_EQUAL")) + { + // Ignore sal_False/True that are directly used as + // arguments to CPPUNIT_ASSERT_EQUAL: + return true; + } + bool b = k == FBK_sal_Bool && name == "sal_True"; + if (rewriter != nullptr) { + auto callSpellLoc = compiler.getSourceManager() + .getSpellingLoc(callLoc); + unsigned n = Lexer::MeasureTokenLength( + callSpellLoc, compiler.getSourceManager(), + compiler.getLangOpts()); + if (StringRef( + compiler.getSourceManager().getCharacterData( + callSpellLoc), + n) + == name) + { + return replaceText( + callSpellLoc, n, b ? "true" : "false"); + } + } + report( + DiagnosticsEngine::Warning, + "use '%select{false|true}0' instead of '%1'", callLoc) + << b << name << expr->getSourceRange(); + } + return true; + } + } + report( + DiagnosticsEngine::Warning, + "CStyleCastExpr, suspicious cast from %0 to %1", + compat::getBeginLoc(expr)) + << expr->getSubExpr()->IgnoreParenImpCasts()->getType() + << expr->getType() << expr->getSourceRange(); + } + return true; +} + +bool FakeBool::VisitCXXStaticCastExpr(CXXStaticCastExpr * expr) { + if (ignoreLocation(expr)) { + return true; + } + auto const k = isFakeBool(expr->getType()); + if (k == FBK_No) { + return true; + } + if (k == FBK_sal_Bool + && isInSpecialMainFile( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(expr)))) + { + return true; + } + report( + DiagnosticsEngine::Warning, + "CXXStaticCastExpr, suspicious cast from %0 to %1", + compat::getBeginLoc(expr)) + << expr->getSubExpr()->IgnoreParenImpCasts()->getType() + << expr->getType() << expr->getSourceRange(); + return true; +} + +bool FakeBool::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr) { + if (ignoreLocation(expr)) { + return true; + } + if (isFakeBool(expr->getType()) != FBK_No) { + report( + DiagnosticsEngine::Warning, + "CXXFunctionalCastExpr, suspicious cast from %0 to %1", + compat::getBeginLoc(expr)) + << expr->getSubExpr()->IgnoreParenImpCasts()->getType() + << expr->getType() << expr->getSourceRange(); + } + return true; +} + +bool FakeBool::VisitImplicitCastExpr(ImplicitCastExpr * expr) { + FunctionAddress::VisitImplicitCastExpr(expr); + if (ignoreLocation(expr)) { + return true; + } + auto const k = isFakeBool(expr->getType()); + if (k == FBK_No) { + return true; + } + auto l = compat::getBeginLoc(expr); + while (compiler.getSourceManager().isMacroArgExpansion(l)) { + l = compiler.getSourceManager().getImmediateMacroCallerLoc(l); + } + if (compiler.getSourceManager().isMacroBodyExpansion(l)) { + auto n = Lexer::getImmediateMacroName( + l, compiler.getSourceManager(), compiler.getLangOpts()); + if ((k == FBK_GLboolean && (n == "GL_FALSE" || n == "GL_TRUE")) + || (k == FBK_UBool && (n == "FALSE" || n == "TRUE")) + || (k == FBK_jboolean && (n == "JNI_FALSE" || n == "JNI_TRUE")) + || (k == FBK_sal_Bool && (n == "sal_False" || n == "sal_True"))) + { + return true; + } + } + auto e1 = expr->getSubExprAsWritten(); + auto t = e1->getType(); + if (!t->isFundamentalType() || loplugin::TypeCheck(t).AnyBoolean()) { + return true; + } + auto e2 = dyn_cast(e1); + if (e2 != nullptr) { + auto ic1 = dyn_cast( + e2->getTrueExpr()->IgnoreParens()); + auto ic2 = dyn_cast( + e2->getFalseExpr()->IgnoreParens()); + if (ic1 != nullptr && ic2 != nullptr + && ic1->getType()->isSpecificBuiltinType(BuiltinType::Int) + && (loplugin::TypeCheck(ic1->getSubExprAsWritten()->getType()) + .AnyBoolean()) + && ic2->getType()->isSpecificBuiltinType(BuiltinType::Int) + && (loplugin::TypeCheck(ic2->getSubExprAsWritten()->getType()) + .AnyBoolean())) + { + return true; + } + } + report( + DiagnosticsEngine::Warning, "conversion from %0 to %1", + compat::getBeginLoc(expr)) + << t << expr->getType() << expr->getSourceRange(); + return true; +} + +bool FakeBool::VisitReturnStmt(ReturnStmt const * stmt) { + // Just enough to avoid warnings in rtl_getUriCharClass (sal/rtl/uri.cxx), + // which has + // + // static sal_Bool const aCharClass[][nCharClassSize] = ...; + // + // and + // + // return aCharClass[eCharClass]; + // + if (ignoreLocation(stmt)) { + return true; + } + auto e = stmt->getRetValue(); + if (e == nullptr) { + return true; + } + auto t = e->getType(); + if (!t->isPointerType()) { + return true; + } + for (;;) { + auto t2 = t->getAs(); + if (t2 == nullptr) { + break; + } + t = t2->getPointeeType(); + } + if (isFakeBool(t) != FBK_sal_Bool) { + return true; + } + auto e2 = dyn_cast(e->IgnoreParenImpCasts()); + if (e2 == nullptr) { + return true; + } + auto e3 = dyn_cast(e2->getBase()->IgnoreParenImpCasts()); + if (e3 == nullptr) { + return true; + } + auto d = dyn_cast(e3->getDecl()); + if (d == nullptr) { + return true; + } + varDecls_.erase(d); + return true; +} + +bool FakeBool::WalkUpFromParmVarDecl(ParmVarDecl const * decl) { + return VisitParmVarDecl(decl); +} + +bool FakeBool::VisitParmVarDecl(ParmVarDecl const * decl) { + if (ignoreLocation(decl)) { + return true; + } + auto const fbk = isFakeBool(decl->getType().getNonReferenceType()); + if (fbk != FBK_No) { + FunctionDecl const * f = dyn_cast(decl->getDeclContext()); + if (f != nullptr) { // e.g.: typedef sal_Bool (* FuncPtr )( sal_Bool ); + f = f->getCanonicalDecl(); + if (isAllRelevantCodeDefined(f) + && !(hasCLanguageLinkageType(f) + || (fbk == FBK_sal_Bool && isInUnoIncludeFile(f) + && (!f->isInlined() || f->hasAttr() + || decl->getType()->isReferenceType() + || hasBoolOverload(f, false))) + || f->isDeleted() || hasBoolOverload(f, true))) + { + OverrideKind k = getOverrideKind(f); + if (k != OverrideKind::YES) { + parmVarDecls_.insert({decl, fbk}); + } + } + } + } + return true; +} + +bool FakeBool::WalkUpFromVarDecl(VarDecl const * decl) { + return VisitVarDecl(decl); +} + +bool FakeBool::VisitVarDecl(VarDecl const * decl) { + if (ignoreLocation(decl)) { + return true; + } + if (decl->isExternC()) { + return true; + } + auto k = isFakeBool(decl->getType()); + if (k == FBK_No) { + k = isFakeBoolArray(decl->getType()); + } + if (k == FBK_No) { + return true; + } + auto const loc = compat::getBeginLoc(decl); + if (k == FBK_sal_Bool + && isInSpecialMainFile( + compiler.getSourceManager().getSpellingLoc(loc))) + { + return true; + } + auto l = loc; + while (compiler.getSourceManager().isMacroArgExpansion(l)) { + l = compiler.getSourceManager().getImmediateMacroCallerLoc(l); + } + if (compiler.getSourceManager().isMacroBodyExpansion(l) + && isSharedCAndCppCode(compiler.getSourceManager().getImmediateMacroCallerLoc(l))) + { + return true; + } + varDecls_.insert({decl, k}); + return true; +} + +bool FakeBool::WalkUpFromFieldDecl(FieldDecl const * decl) { + return VisitFieldDecl(decl); +} + +bool FakeBool::VisitFieldDecl(FieldDecl const * decl) { + if (ignoreLocation(decl)) { + return true; + } + auto k = isFakeBool(decl->getType()); + if (k == FBK_No) { + k = isFakeBoolArray(decl->getType()); + } + if (k == FBK_No) { + return true; + } + if (!isAllRelevantCodeDefined(decl)) { + return true; + } + if (k == FBK_sal_Bool + && isInSpecialMainFile( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(decl)))) + { + return true; + } + TagDecl const * td = dyn_cast(decl->getDeclContext()); + assert(td != nullptr); + if (!(((td->isStruct() || td->isUnion()) && td->isExternCContext()) + || isInUnoIncludeFile( + compiler.getSourceManager().getSpellingLoc( + decl->getLocation())))) + { + fieldDecls_.insert({decl, k}); + } + return true; +} + +bool FakeBool::WalkUpFromFunctionDecl(FunctionDecl const * decl) { + return VisitFunctionDecl(decl); +} + +bool FakeBool::VisitFunctionDecl(FunctionDecl const * decl) { + if (ignoreLocation(decl)) { + return true; + } + auto const fbk = isFakeBool(decl->getReturnType().getNonReferenceType()); + if (fbk != FBK_No + && !(decl->isDeletedAsWritten() && isa(decl)) + && isAllRelevantCodeDefined(decl)) + { + FunctionDecl const * f = decl->getCanonicalDecl(); + OverrideKind k = getOverrideKind(f); + if (k != OverrideKind::YES + && !(hasCLanguageLinkageType(f) + || (isInUnoIncludeFile(f) + && (!f->isInlined() || f->hasAttr())))) + { + functionDecls_.insert({decl, fbk}); + } + } + return true; +} + +bool FakeBool::VisitValueDecl(ValueDecl const * decl) { + if (ignoreLocation(decl)) { + return true; + } + auto const k = isFakeBool(decl->getType()); + if (k != FBK_No && !rewrite(compat::getBeginLoc(decl), k)) { + report( + DiagnosticsEngine::Warning, + "ValueDecl, use \"bool\" instead of %0", + compat::getBeginLoc(decl)) + << decl->getType() << decl->getSourceRange(); + } + return true; +} + +bool FakeBool::TraverseStaticAssertDecl(StaticAssertDecl * decl) { + // Ignore special code like + // + // static_cast(true) == sal_True + // + // inside static_assert in cppu/source/uno/check.cxx: + return + loplugin::isSamePathname( + getFilenameOfLocation(decl->getLocation()), + SRCDIR "/cppu/source/uno/check.cxx") + || RecursiveASTVisitor::TraverseStaticAssertDecl(decl); +} + +bool FakeBool::TraverseLinkageSpecDecl(LinkageSpecDecl * decl) { + assert(externCContexts_ != std::numeric_limits::max()); //TODO + ++externCContexts_; + bool ret = RecursiveASTVisitor::TraverseLinkageSpecDecl(decl); + assert(externCContexts_ != 0); + --externCContexts_; + return ret; +} + +bool FakeBool::isFromCIncludeFile(SourceLocation spellingLocation) const { + return !compiler.getSourceManager().isInMainFile(spellingLocation) + && (StringRef( + compiler.getSourceManager().getPresumedLoc(spellingLocation) + .getFilename()) + .endswith(".h")); +} + +bool FakeBool::isSharedCAndCppCode(SourceLocation location) const { + // Assume that code is intended to be shared between C and C++ if it comes + // from an include file ending in .h, and is either in an extern "C" context + // or the body of a macro definition: + return + isFromCIncludeFile(compiler.getSourceManager().getSpellingLoc(location)) + && (externCContexts_ != 0 + || compiler.getSourceManager().isMacroBodyExpansion(location)); +} + +bool FakeBool::isInSpecialMainFile(SourceLocation spellingLocation) const { + if (!compiler.getSourceManager().isInMainFile(spellingLocation)) { + return false; + } + auto f = getFilenameOfLocation(spellingLocation); + return loplugin::isSamePathname(f, SRCDIR "/cppu/qa/test_any.cxx") + || loplugin::isSamePathname(f, SRCDIR "/cppu/source/uno/check.cxx"); + // TODO: the offset checks +} + +bool FakeBool::rewrite(SourceLocation location, FakeBoolKind kind) { + if (rewriter != nullptr) { + //TODO: "::sal_Bool" -> "bool", not "::bool" + SourceLocation loc { compiler.getSourceManager().getExpansionLoc( + location) }; + unsigned n = Lexer::MeasureTokenLength( + loc, compiler.getSourceManager(), compiler.getLangOpts()); + if (std::string(compiler.getSourceManager().getCharacterData(loc), n) + == getName(kind)) + { + return replaceText(loc, n, "bool"); + } + } + return false; +} + +loplugin::Plugin::Registration X("fakebool", true); + +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/salbool.cxx b/compilerplugins/clang/salbool.cxx deleted file mode 100644 index fdadfc6b795a..000000000000 --- a/compilerplugins/clang/salbool.cxx +++ /dev/null @@ -1,832 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* - * This file is part of the LibreOffice project. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -#include -#include -#include -#include -#include - -#include "clang/AST/Attr.h" - -#include "check.hxx" -#include "compat.hxx" -#include "plugin.hxx" - -namespace { - -bool isSalBool(QualType type) { - TypedefType const * t = type->getAs(); - return t != nullptr && t->getDecl()->getName() == "sal_Bool"; -} - -bool isSalBoolArray(QualType type) { - auto t = type->getAsArrayTypeUnsafe(); - return t != nullptr - && (isSalBool(t->getElementType()) - || isSalBoolArray(t->getElementType())); -} - -// It appears that, given a function declaration, there is no way to determine -// the language linkage of the function's type, only of the function's name -// (via FunctionDecl::isExternC); however, in a case like -// -// extern "C" { static void f(); } -// -// the function's name does not have C language linkage while the function's -// type does (as clarified in C++11 [decl.link]); cf. -// "Language linkage of function type": -bool hasCLanguageLinkageType(FunctionDecl const * decl) { - assert(decl != nullptr); - if (decl->isExternC()) { - return true; - } - if (decl->isInExternCContext()) { - return true; - } - return false; -} - -enum class OverrideKind { NO, YES, MAYBE }; - -OverrideKind getOverrideKind(FunctionDecl const * decl) { - CXXMethodDecl const * m = dyn_cast(decl); - if (m == nullptr) { - return OverrideKind::NO; - } - if (m->size_overridden_methods() != 0 || m->hasAttr()) { - return OverrideKind::YES; - } - if (!dyn_cast(m->getDeclContext())->hasAnyDependentBases()) { - return OverrideKind::NO; - } - return OverrideKind::MAYBE; -} - -enum class BoolOverloadKind { No, Yes, CheckNext }; - -BoolOverloadKind isBoolOverloadOf( - FunctionDecl const * f, FunctionDecl const * decl, bool mustBeDeleted) -{ - if (!mustBeDeleted || f->isDeleted()) { - unsigned n = decl->getNumParams(); - if (f->getNumParams() == n) { - bool hasSB = false; - for (unsigned i = 0; i != n; ++i) { - QualType t1 { decl->getParamDecl(i)->getType() }; - bool isSB = isSalBool(t1); - bool isSBRef = !isSB && t1->isReferenceType() - && isSalBool(t1.getNonReferenceType()); - QualType t2 { f->getParamDecl(i)->getType() }; - if (!(isSB - ? t2->isBooleanType() - : isSBRef - ? (t2->isReferenceType() - && t2.getNonReferenceType()->isBooleanType()) - : t2.getCanonicalType() == t1.getCanonicalType())) - { - return BoolOverloadKind::CheckNext; - } - hasSB |= isSB || isSBRef; - } - return hasSB ? BoolOverloadKind::Yes : BoolOverloadKind::No; - // cheaply protect against the case where decl would have no - // sal_Bool parameters at all and would match itself - } - } - return BoolOverloadKind::CheckNext; -} - -//TODO: current implementation is not at all general, just tests what we -// encounter in practice: -bool hasBoolOverload(FunctionDecl const * decl, bool mustBeDeleted) { - auto ctx = decl->getDeclContext(); - if (!ctx->isLookupContext()) { - return false; - } - auto res = ctx->lookup(decl->getDeclName()); - for (auto d = res.begin(); d != res.end(); ++d) { - if (auto f = dyn_cast(*d)) { - switch (isBoolOverloadOf(f, decl, mustBeDeleted)) { - case BoolOverloadKind::No: - return false; - case BoolOverloadKind::Yes: - return true; - case BoolOverloadKind::CheckNext: - break; - } - } else if (auto ftd = dyn_cast(*d)) { - for (auto f: ftd->specializations()) { - if (f->getTemplateSpecializationKind() - == TSK_ExplicitSpecialization) - { - switch (isBoolOverloadOf(f, decl, mustBeDeleted)) { - case BoolOverloadKind::No: - return false; - case BoolOverloadKind::Yes: - return true; - case BoolOverloadKind::CheckNext: - break; - } - } - } - } - } - return false; -} - -class SalBool: - public loplugin::FilteringRewritePlugin -{ -public: - explicit SalBool(loplugin::InstantiationData const & data): - FilteringRewritePlugin(data) {} - - virtual void run() override; - - bool VisitUnaryAddrOf(UnaryOperator const * op); - - bool VisitCallExpr(CallExpr * expr); - - bool VisitCStyleCastExpr(CStyleCastExpr * expr); - - bool VisitCXXStaticCastExpr(CXXStaticCastExpr * expr); - - bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr); - - bool VisitImplicitCastExpr(ImplicitCastExpr * expr); - - bool VisitReturnStmt(ReturnStmt const * stmt); - - bool WalkUpFromParmVarDecl(ParmVarDecl const * decl); - bool VisitParmVarDecl(ParmVarDecl const * decl); - - bool WalkUpFromVarDecl(VarDecl const * decl); - bool VisitVarDecl(VarDecl const * decl); - - bool WalkUpFromFieldDecl(FieldDecl const * decl); - bool VisitFieldDecl(FieldDecl const * decl); - - bool WalkUpFromFunctionDecl(FunctionDecl const * decl); - bool VisitFunctionDecl(FunctionDecl const * decl); - - bool VisitValueDecl(ValueDecl const * decl); - - bool TraverseStaticAssertDecl(StaticAssertDecl * decl); - - bool TraverseLinkageSpecDecl(LinkageSpecDecl * decl); - -private: - bool isFromCIncludeFile(SourceLocation spellingLocation) const; - - bool isSharedCAndCppCode(SourceLocation location) const; - - bool isInSpecialMainFile(SourceLocation spellingLocation) const; - - bool rewrite(SourceLocation location); - - std::set varDecls_; - unsigned int externCContexts_ = 0; -}; - -void SalBool::run() { - if (compiler.getLangOpts().CPlusPlus) { - TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); - for (auto decl: varDecls_) { - SourceLocation loc { compat::getBeginLoc(decl) }; - TypeSourceInfo * tsi = decl->getTypeSourceInfo(); - if (tsi != nullptr) { - SourceLocation l { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getBeginLoc()) }; - SourceLocation end { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getEndLoc()) }; - assert(l.isFileID() && end.isFileID()); - if (l == end - || compiler.getSourceManager().isBeforeInTranslationUnit( - l, end)) - { - for (;;) { - unsigned n = Lexer::MeasureTokenLength( - l, compiler.getSourceManager(), - compiler.getLangOpts()); - std::string s { - compiler.getSourceManager().getCharacterData(l), - n }; - if (s == "sal_Bool") { - loc = l; - break; - } - if (l == end) { - break; - } - l = l.getLocWithOffset(std::max(n, 1)); - } - } - } - if (!rewrite(loc)) { - report( - DiagnosticsEngine::Warning, - "VarDecl, use \"bool\" instead of \"sal_Bool\"", loc) - << decl->getSourceRange(); - } - } - } -} - -bool SalBool::VisitUnaryAddrOf(UnaryOperator const * op) { - Expr const * e1 = op->getSubExpr()->IgnoreParenCasts(); - if (isSalBool(e1->getType())) { - DeclRefExpr const * e2 = dyn_cast(e1); - if (e2 != nullptr) { - VarDecl const * d = dyn_cast(e2->getDecl()); - if (d != nullptr) { - varDecls_.erase(d); - } - } - } - return true; -} - -bool SalBool::VisitCallExpr(CallExpr * expr) { - Decl const * d = expr->getCalleeDecl(); - FunctionProtoType const * ft = nullptr; - if (d != nullptr) { - FunctionDecl const * fd = dyn_cast(d); - if (fd != nullptr) { - if (!hasBoolOverload(fd, false)) { - clang::PointerType const * pt = fd->getType() - ->getAs(); - QualType t2( - pt == nullptr ? fd->getType() : pt->getPointeeType()); - ft = t2->getAs(); - assert( - ft != nullptr || !compiler.getLangOpts().CPlusPlus - || (fd->getBuiltinID() != Builtin::NotBuiltin - && isa(t2))); - // __builtin_*s have no proto type? - } - } else { - VarDecl const * vd = dyn_cast(d); - if (vd != nullptr) { - clang::PointerType const * pt = vd->getType() - ->getAs(); - ft = (pt == nullptr ? vd->getType() : pt->getPointeeType()) - ->getAs(); - } - } - } - if (ft != nullptr) { - for (unsigned i = 0; i != ft->getNumParams(); ++i) { - QualType t(ft->getParamType(i)); - bool b = false; - if (t->isLValueReferenceType()) { - t = t.getNonReferenceType(); - b = !t.isConstQualified() && isSalBool(t); - } else if (t->isPointerType()) { - for (;;) { - auto t2 = t->getAs(); - if (t2 == nullptr) { - break; - } - t = t2->getPointeeType(); - } - b = isSalBool(t); - } - if (b && i < expr->getNumArgs()) { - DeclRefExpr * ref = dyn_cast( - expr->getArg(i)->IgnoreParenImpCasts()); - if (ref != nullptr) { - VarDecl const * d = dyn_cast(ref->getDecl()); - if (d != nullptr) { - varDecls_.erase(d); - } - } - } - } - } - return true; -} - -bool SalBool::VisitCStyleCastExpr(CStyleCastExpr * expr) { - if (ignoreLocation(expr)) { - return true; - } - if (isSalBool(expr->getType())) { - SourceLocation loc { compat::getBeginLoc(expr) }; - while (compiler.getSourceManager().isMacroArgExpansion(loc)) { - loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc); - } - if (compiler.getSourceManager().isMacroBodyExpansion(loc)) { - StringRef name { Lexer::getImmediateMacroName( - loc, compiler.getSourceManager(), compiler.getLangOpts()) }; - if (name == "sal_False" || name == "sal_True") { - auto callLoc = compiler.getSourceManager() - .getImmediateMacroCallerLoc(loc); - if (!isSharedCAndCppCode(callLoc)) { - SourceLocation argLoc; - if (compiler.getSourceManager().isMacroArgExpansion( - compat::getBeginLoc(expr), &argLoc) - //TODO: check it's the complete (first) arg to the macro - && (Lexer::getImmediateMacroName( - argLoc, compiler.getSourceManager(), - compiler.getLangOpts()) - == "CPPUNIT_ASSERT_EQUAL")) - { - // Ignore sal_False/True that are directly used as - // arguments to CPPUNIT_ASSERT_EQUAL: - return true; - } - bool b = name == "sal_True"; - if (rewriter != nullptr) { - auto callSpellLoc = compiler.getSourceManager() - .getSpellingLoc(callLoc); - unsigned n = Lexer::MeasureTokenLength( - callSpellLoc, compiler.getSourceManager(), - compiler.getLangOpts()); - if (StringRef( - compiler.getSourceManager().getCharacterData( - callSpellLoc), - n) - == name) - { - return replaceText( - callSpellLoc, n, b ? "true" : "false"); - } - } - report( - DiagnosticsEngine::Warning, - "use '%select{false|true}0' instead of '%1'", callLoc) - << b << name << expr->getSourceRange(); - } - return true; - } - } - report( - DiagnosticsEngine::Warning, - "CStyleCastExpr, suspicious cast from %0 to %1", - compat::getBeginLoc(expr)) - << expr->getSubExpr()->IgnoreParenImpCasts()->getType() - << expr->getType() << expr->getSourceRange(); - } - return true; -} - -bool SalBool::VisitCXXStaticCastExpr(CXXStaticCastExpr * expr) { - if (ignoreLocation(expr)) { - return true; - } - if (isSalBool(expr->getType()) - && !isInSpecialMainFile( - compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(expr)))) - { - report( - DiagnosticsEngine::Warning, - "CXXStaticCastExpr, suspicious cast from %0 to %1", - compat::getBeginLoc(expr)) - << expr->getSubExpr()->IgnoreParenImpCasts()->getType() - << expr->getType() << expr->getSourceRange(); - } - return true; -} - -bool SalBool::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr) { - if (ignoreLocation(expr)) { - return true; - } - if (isSalBool(expr->getType())) { - report( - DiagnosticsEngine::Warning, - "CXXFunctionalCastExpr, suspicious cast from %0 to %1", - compat::getBeginLoc(expr)) - << expr->getSubExpr()->IgnoreParenImpCasts()->getType() - << expr->getType() << expr->getSourceRange(); - } - return true; -} - -bool SalBool::VisitImplicitCastExpr(ImplicitCastExpr * expr) { - if (ignoreLocation(expr)) { - return true; - } - if (!isSalBool(expr->getType())) { - return true; - } - auto l = compat::getBeginLoc(expr); - while (compiler.getSourceManager().isMacroArgExpansion(l)) { - l = compiler.getSourceManager().getImmediateMacroCallerLoc(l); - } - if (compiler.getSourceManager().isMacroBodyExpansion(l)) { - auto n = Lexer::getImmediateMacroName( - l, compiler.getSourceManager(), compiler.getLangOpts()); - if (n == "sal_False" || n == "sal_True") { - return true; - } - } - auto e1 = expr->getSubExprAsWritten(); - auto t = e1->getType(); - if (!t->isFundamentalType() || loplugin::TypeCheck(t).AnyBoolean()) { - return true; - } - auto e2 = dyn_cast(e1); - if (e2 != nullptr) { - auto ic1 = dyn_cast( - e2->getTrueExpr()->IgnoreParens()); - auto ic2 = dyn_cast( - e2->getFalseExpr()->IgnoreParens()); - if (ic1 != nullptr && ic2 != nullptr - && ic1->getType()->isSpecificBuiltinType(BuiltinType::Int) - && (loplugin::TypeCheck(ic1->getSubExprAsWritten()->getType()) - .AnyBoolean()) - && ic2->getType()->isSpecificBuiltinType(BuiltinType::Int) - && (loplugin::TypeCheck(ic2->getSubExprAsWritten()->getType()) - .AnyBoolean())) - { - return true; - } - } - report( - DiagnosticsEngine::Warning, "conversion from %0 to sal_Bool", - compat::getBeginLoc(expr)) - << t << expr->getSourceRange(); - return true; -} - -bool SalBool::VisitReturnStmt(ReturnStmt const * stmt) { - // Just enough to avoid warnings in rtl_getUriCharClass (sal/rtl/uri.cxx), - // which has - // - // static sal_Bool const aCharClass[][nCharClassSize] = ...; - // - // and - // - // return aCharClass[eCharClass]; - // - if (ignoreLocation(stmt)) { - return true; - } - auto e = stmt->getRetValue(); - if (e == nullptr) { - return true; - } - auto t = e->getType(); - if (!t->isPointerType()) { - return true; - } - for (;;) { - auto t2 = t->getAs(); - if (t2 == nullptr) { - break; - } - t = t2->getPointeeType(); - } - if (!isSalBool(t)) { - return true; - } - auto e2 = dyn_cast(e->IgnoreParenImpCasts()); - if (e2 == nullptr) { - return true; - } - auto e3 = dyn_cast(e2->getBase()->IgnoreParenImpCasts()); - if (e3 == nullptr) { - return true; - } - auto d = dyn_cast(e3->getDecl()); - if (d == nullptr) { - return true; - } - varDecls_.erase(d); - return true; -} - -bool SalBool::WalkUpFromParmVarDecl(ParmVarDecl const * decl) { - return VisitParmVarDecl(decl); -} - -bool SalBool::VisitParmVarDecl(ParmVarDecl const * decl) { - if (ignoreLocation(decl)) { - return true; - } - if (isSalBool(decl->getType().getNonReferenceType())) { - FunctionDecl const * f = dyn_cast(decl->getDeclContext()); - if (f != nullptr) { // e.g.: typedef sal_Bool (* FuncPtr )( sal_Bool ); - f = f->getCanonicalDecl(); - if (!(hasCLanguageLinkageType(f) - || (isInUnoIncludeFile(f) - && (!f->isInlined() || f->hasAttr() - || decl->getType()->isReferenceType() - || hasBoolOverload(f, false))) - || f->isDeleted() || hasBoolOverload(f, true))) - { - OverrideKind k = getOverrideKind(f); - if (k != OverrideKind::YES) { - SourceLocation loc { compat::getBeginLoc(decl) }; - TypeSourceInfo * tsi = decl->getTypeSourceInfo(); - if (tsi != nullptr) { - SourceLocation l { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getBeginLoc()) }; - SourceLocation end { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getEndLoc()) }; - assert(l.isFileID() && end.isFileID()); - if (l == end - || (compiler.getSourceManager() - .isBeforeInTranslationUnit(l, end))) - { - for (;;) { - unsigned n = Lexer::MeasureTokenLength( - l, compiler.getSourceManager(), - compiler.getLangOpts()); - std::string s { - compiler.getSourceManager().getCharacterData(l), - n }; - if (s == "sal_Bool") { - loc = l; - break; - } - if (l == end) { - break; - } - l = l.getLocWithOffset(std::max(n, 1)); - } - } - } - // Only rewrite declarations in include files if a - // definition is also seen, to avoid compilation of a - // definition (in a main file only processed later) to fail - // with a "mismatch" error before the rewriter had a chance - // to act upon the definition (but use the heuristic of - // assuming pure virtual functions do not have definitions); - // also, do not automatically rewrite functions that could - // implicitly override depend base functions (and thus stop - // doing so after the rewrite; note that this is less - // dangerous for return types than for parameter types, - // where the function would still implicitly override and - // cause a compilation error due to the incompatible return - // type): - if (!((compiler.getSourceManager().isInMainFile( - compiler.getSourceManager().getSpellingLoc( - dyn_cast( - decl->getDeclContext()) - ->getNameInfo().getLoc())) - || f->isDefined() || f->isPure()) - && k != OverrideKind::MAYBE && rewrite(loc))) - { - report( - DiagnosticsEngine::Warning, - ("ParmVarDecl, use \"bool\" instead of" - " \"sal_Bool\"%0"), - loc) - << (k == OverrideKind::MAYBE - ? (" (unless this member function overrides a" - " dependent base member function, even" - " though it is not marked 'override')") - : "") - << decl->getSourceRange(); - } - } - } - } - } - return true; -} - -bool SalBool::WalkUpFromVarDecl(VarDecl const * decl) { - return VisitVarDecl(decl); -} - -bool SalBool::VisitVarDecl(VarDecl const * decl) { - if (ignoreLocation(decl)) { - return true; - } - if (!decl->isExternC() - && (isSalBool(decl->getType()) || isSalBoolArray(decl->getType())) - && !isInSpecialMainFile( - compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(decl)))) - { - varDecls_.insert(decl); - } - return true; -} - -bool SalBool::WalkUpFromFieldDecl(FieldDecl const * decl) { - return VisitFieldDecl(decl); -} - -bool SalBool::VisitFieldDecl(FieldDecl const * decl) { - if (ignoreLocation(decl)) { - return true; - } - if ((isSalBool(decl->getType()) || isSalBoolArray(decl->getType())) - && !isInSpecialMainFile( - compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(decl)))) - { - TagDecl const * td = dyn_cast(decl->getDeclContext()); - assert(td != nullptr); - if (!(((td->isStruct() || td->isUnion()) && td->isExternCContext()) - || isInUnoIncludeFile( - compiler.getSourceManager().getSpellingLoc( - decl->getLocation())))) - { - SourceLocation loc { compat::getBeginLoc(decl) }; - TypeSourceInfo * tsi = decl->getTypeSourceInfo(); - if (tsi != nullptr) { - SourceLocation l { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getBeginLoc()) }; - SourceLocation end { - compiler.getSourceManager().getExpansionLoc( - tsi->getTypeLoc().getEndLoc()) }; - assert(l.isFileID() && end.isFileID()); - if (l == end - || compiler.getSourceManager().isBeforeInTranslationUnit( - l, end)) - { - for (;;) { - unsigned n = Lexer::MeasureTokenLength( - l, compiler.getSourceManager(), - compiler.getLangOpts()); - std::string s { - compiler.getSourceManager().getCharacterData(l), - n }; - if (s == "sal_Bool") { - loc = l; - break; - } - if (l == end) { - break; - } - l = l.getLocWithOffset(std::max(n, 1)); - } - } - } - if (!rewrite(loc)) { - report( - DiagnosticsEngine::Warning, - "FieldDecl, use \"bool\" instead of \"sal_Bool\"", loc) - << decl->getSourceRange(); - } - } - } - return true; -} - -bool SalBool::WalkUpFromFunctionDecl(FunctionDecl const * decl) { - return VisitFunctionDecl(decl); -} - -bool SalBool::VisitFunctionDecl(FunctionDecl const * decl) { - if (ignoreLocation(decl)) { - return true; - } - if (isSalBool(decl->getReturnType().getNonReferenceType()) - && !(decl->isDeletedAsWritten() && isa(decl))) - { - FunctionDecl const * f = decl->getCanonicalDecl(); - OverrideKind k = getOverrideKind(f); - if (k != OverrideKind::YES - && !(hasCLanguageLinkageType(f) - || (isInUnoIncludeFile(f) - && (!f->isInlined() || f->hasAttr())))) - { - SourceLocation loc { compat::getBeginLoc(decl) }; - SourceLocation l { compiler.getSourceManager().getExpansionLoc( - loc) }; - SourceLocation end { compiler.getSourceManager().getExpansionLoc( - decl->getNameInfo().getLoc()) }; - assert(l.isFileID() && end.isFileID()); - if (compiler.getSourceManager().isBeforeInTranslationUnit(l, end)) { - while (l != end) { - unsigned n = Lexer::MeasureTokenLength( - l, compiler.getSourceManager(), compiler.getLangOpts()); - std::string s { - compiler.getSourceManager().getCharacterData(l), n }; - if (s == "sal_Bool") { - loc = l; - break; - } - l = l.getLocWithOffset(std::max(n, 1)); - } - } - // Only rewrite declarations in include files if a definition is - // also seen, to avoid compilation of a definition (in a main file - // only processed later) to fail with a "mismatch" error before the - // rewriter had a chance to act upon the definition (but use the - // heuristic of assuming pure virtual functions do not have - // definitions): - if (!((compiler.getSourceManager().isInMainFile( - compiler.getSourceManager().getSpellingLoc( - decl->getNameInfo().getLoc())) - || f->isDefined() || f->isPure()) - && rewrite(loc))) - { - report( - DiagnosticsEngine::Warning, - "use \"bool\" instead of \"sal_Bool\" as return type%0", - loc) - << (k == OverrideKind::MAYBE - ? (" (unless this member function overrides a dependent" - " base member function, even though it is not marked" - " 'override')") - : "") - << decl->getSourceRange(); - } - } - } - return true; -} - -bool SalBool::VisitValueDecl(ValueDecl const * decl) { - if (ignoreLocation(decl)) { - return true; - } - if (isSalBool(decl->getType()) && !rewrite(compat::getBeginLoc(decl))) { - report( - DiagnosticsEngine::Warning, - "ValueDecl, use \"bool\" instead of \"sal_Bool\"", - compat::getBeginLoc(decl)) - << decl->getSourceRange(); - } - return true; -} - -bool SalBool::TraverseStaticAssertDecl(StaticAssertDecl * decl) { - // Ignore special code like - // - // static_cast(true) == sal_True - // - // inside static_assert in cppu/source/uno/check.cxx: - return - loplugin::isSamePathname( - getFilenameOfLocation(decl->getLocation()), - SRCDIR "/cppu/source/uno/check.cxx") - || RecursiveASTVisitor::TraverseStaticAssertDecl(decl); -} - -bool SalBool::TraverseLinkageSpecDecl(LinkageSpecDecl * decl) { - assert(externCContexts_ != std::numeric_limits::max()); //TODO - ++externCContexts_; - bool ret = RecursiveASTVisitor::TraverseLinkageSpecDecl(decl); - assert(externCContexts_ != 0); - --externCContexts_; - return ret; -} - -bool SalBool::isFromCIncludeFile(SourceLocation spellingLocation) const { - return !compiler.getSourceManager().isInMainFile(spellingLocation) - && (StringRef( - compiler.getSourceManager().getPresumedLoc(spellingLocation) - .getFilename()) - .endswith(".h")); -} - -bool SalBool::isSharedCAndCppCode(SourceLocation location) const { - // Assume that code is intended to be shared between C and C++ if it comes - // from an include file ending in .h, and is either in an extern "C" context - // or the body of a macro definition: - return - isFromCIncludeFile(compiler.getSourceManager().getSpellingLoc(location)) - && (externCContexts_ != 0 - || compiler.getSourceManager().isMacroBodyExpansion(location)); -} - -bool SalBool::isInSpecialMainFile(SourceLocation spellingLocation) const { - if (!compiler.getSourceManager().isInMainFile(spellingLocation)) { - return false; - } - auto f = getFilenameOfLocation(spellingLocation); - return loplugin::isSamePathname(f, SRCDIR "/cppu/qa/test_any.cxx") - || loplugin::isSamePathname(f, SRCDIR "/cppu/source/uno/check.cxx"); - // TODO: the offset checks -} - -bool SalBool::rewrite(SourceLocation location) { - if (rewriter != nullptr) { - //TODO: "::sal_Bool" -> "bool", not "::bool" - SourceLocation loc { compiler.getSourceManager().getExpansionLoc( - location) }; - unsigned n = Lexer::MeasureTokenLength( - loc, compiler.getSourceManager(), compiler.getLangOpts()); - if (std::string(compiler.getSourceManager().getCharacterData(loc), n) - == "sal_Bool") - { - return replaceText(loc, n, "bool"); - } - } - return false; -} - -loplugin::Plugin::Registration X("salbool", true); - -} - -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/fakebool.cxx b/compilerplugins/clang/test/fakebool.cxx new file mode 100644 index 000000000000..26b5d7e2f791 --- /dev/null +++ b/compilerplugins/clang/test/fakebool.cxx @@ -0,0 +1,22 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include + +#include + +namespace { + +struct S { + sal_Bool b; // expected-error {{FieldDecl, use "bool" instead of 'sal_Bool' (aka 'unsigned char') [loplugin:fakebool]}} +}; + +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/test/salbool.cxx b/compilerplugins/clang/test/salbool.cxx deleted file mode 100644 index da861afe73be..000000000000 --- a/compilerplugins/clang/test/salbool.cxx +++ /dev/null @@ -1,18 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ -/* - * This file is part of the LibreOffice project. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -#include - -#include - -struct S { - sal_Bool b; // expected-error {{FieldDecl, use "bool" instead of "sal_Bool" [loplugin:salbool]}} -}; - -/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ -- cgit v1.2.3