From e8425c48102321d4f5a8bd687c8ca1ac7bae797e Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Wed, 20 Apr 2016 17:23:25 +0200 Subject: loplugin:salbool: Warn about uses of sal_False/True ...that can generally be rewritten as false/true, and sometimes could hide errors, see e.g. <5be5f00fe16b0e255b31fbaba5f119773d1cd071> "So this is apparently about right-to-left levels, not a boolean flag". Change-Id: Ib39a936a632c2aab206f24c346252e31dcbb98f3 --- compilerplugins/clang/salbool.cxx | 61 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/compilerplugins/clang/salbool.cxx b/compilerplugins/clang/salbool.cxx index 0cea0eb5fb66..1c55e346cb98 100644 --- a/compilerplugins/clang/salbool.cxx +++ b/compilerplugins/clang/salbool.cxx @@ -150,7 +150,11 @@ public: bool VisitValueDecl(ValueDecl const * decl); + bool TraverseStaticAssertDecl(StaticAssertDecl * decl); + private: + bool isFromCIncludeFile(SourceLocation spellingLocation) const; + bool isInSpecialMainFile(SourceLocation spellingLocation) const; bool rewrite(SourceLocation location); @@ -276,6 +280,43 @@ bool SalBool::VisitCStyleCastExpr(CStyleCastExpr * expr) { StringRef name { Lexer::getImmediateMacroName( loc, compiler.getSourceManager(), compiler.getLangOpts()) }; if (name == "sal_False" || name == "sal_True") { + auto callLoc = compiler.getSourceManager().getSpellingLoc( + compiler.getSourceManager().getImmediateMacroCallerLoc( + loc)); + if (!isFromCIncludeFile(callLoc)) { + SourceLocation argLoc; + if (compiler.getSourceManager().isMacroArgExpansion( + expr->getLocStart(), &argLoc) + //TODO: check its 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) { + unsigned n = Lexer::MeasureTokenLength( + callLoc, compiler.getSourceManager(), + compiler.getLangOpts()); + if (StringRef( + compiler.getSourceManager().getCharacterData( + callLoc), + n) + == name) + { + return replaceText( + callLoc, n, b ? "true" : "false"); + } + } + report( + DiagnosticsEngine::Warning, + "use '%select{false|true}0' instead of '%1'", callLoc) + << b << name << expr->getSourceRange(); + } return true; } } @@ -578,6 +619,26 @@ bool SalBool::VisitValueDecl(ValueDecl const * decl) { 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 + (compiler.getSourceManager().getFilename(decl->getLocation()) + == SRCDIR "/cppu/source/uno/check.cxx") + || RecursiveASTVisitor::TraverseStaticAssertDecl(decl); +} + +bool SalBool::isFromCIncludeFile(SourceLocation spellingLocation) const { + return !compat::isInMainFile(compiler.getSourceManager(), spellingLocation) + && (StringRef( + compiler.getSourceManager().getPresumedLoc(spellingLocation) + .getFilename()) + .endswith(".h")); +} + bool SalBool::isInSpecialMainFile(SourceLocation spellingLocation) const { if (!compat::isInMainFile(compiler.getSourceManager(), spellingLocation)) { return false; -- cgit