diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2016-04-20 17:23:25 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2016-04-20 17:27:33 +0200 |
commit | e8425c48102321d4f5a8bd687c8ca1ac7bae797e (patch) | |
tree | 3d30552c920139d5c6c56da38d49345f1efae97a | |
parent | f3059ab1dcb4a413f487fbac1329044f9f5a7e90 (diff) |
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
-rw-r--r-- | compilerplugins/clang/salbool.cxx | 61 |
1 files changed, 61 insertions, 0 deletions
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<sal_Bool>(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; |