diff options
| -rw-r--r-- | compilerplugins/clang/cstylecast.cxx | 440 | ||||
| -rw-r--r-- | compilerplugins/clang/test/cstylecast.cxx | 63 | ||||
| -rw-r--r-- | compilerplugins/clang/unnecessaryparen.cxx | 104 | ||||
| -rw-r--r-- | solenv/CompilerTest_compilerplugins_clang.mk | 1 |
4 files changed, 595 insertions, 13 deletions
diff --git a/compilerplugins/clang/cstylecast.cxx b/compilerplugins/clang/cstylecast.cxx index bf8e2fb00809..f09ce81a2987 100644 --- a/compilerplugins/clang/cstylecast.cxx +++ b/compilerplugins/clang/cstylecast.cxx @@ -7,8 +7,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include <algorithm> #include <cassert> #include <limits> +#include <set> #include <string> #include "plugin.hxx" @@ -82,11 +84,89 @@ QualType resolvePointers(QualType type) { return type; } +bool isLiteralLike(Expr const * expr) { + expr = expr->IgnoreParenImpCasts(); + if (isa<IntegerLiteral>(expr) || isa<CharacterLiteral>(expr) || isa<FloatingLiteral>(expr) + || isa<ImaginaryLiteral>(expr) || isa<CXXBoolLiteralExpr>(expr) + || isa<CXXNullPtrLiteralExpr>(expr) || isa<ObjCBoolLiteralExpr>(expr)) + { + return true; + } + if (auto const e = dyn_cast<DeclRefExpr>(expr)) { + auto const d = e->getDecl(); + if (isa<EnumConstantDecl>(d)) { + return true; + } + if (auto const v = dyn_cast<VarDecl>(d)) { + if (d->getType().isConstQualified()) { + if (auto const init = v->getAnyInitializer()) { + return isLiteralLike(init); + } + } + } + return false; + } + if (auto const e = dyn_cast<UnaryExprOrTypeTraitExpr>(expr)) { + auto const k = e->getKind(); + return k == UETT_SizeOf || k == UETT_AlignOf; + } + if (auto const e = dyn_cast<UnaryOperator>(expr)) { + auto const k = e->getOpcode(); + if (k == UO_Plus || k == UO_Minus || k == UO_Not || k == UO_LNot) { + return isLiteralLike(e->getSubExpr()); + } + return false; + } + if (auto const e = dyn_cast<BinaryOperator>(expr)) { + auto const k = e->getOpcode(); + if (k == BO_Mul || k == BO_Div || k == BO_Rem || k == BO_Add || k == BO_Sub || k == BO_Shl + || k == BO_Shr || k == BO_And || k == BO_Xor || k == BO_Or) + { + return isLiteralLike(e->getLHS()) && isLiteralLike(e->getRHS()); + } + return false; + } + if (auto const e = dyn_cast<ExplicitCastExpr>(expr)) { + auto const t = e->getTypeAsWritten(); + return (t->isArithmeticType() || t->isEnumeralType()) + && isLiteralLike(e->getSubExprAsWritten()); + } + return false; +} + +bool canBeUsedForFunctionalCast(TypeSourceInfo const * info) { + // Must be <simple-type-specifier> or <typename-specifier>, lets approximate that here: + assert(info != nullptr); + auto const type = info->getType(); + if (type.hasLocalQualifiers()) { + return false; + } + if (auto const t = dyn_cast<BuiltinType>(type)) { + if (!(t->isInteger() || t->isFloatingPoint())) { + return false; + } + auto const loc = info->getTypeLoc().castAs<BuiltinTypeLoc>(); + return + (int(loc.hasWrittenSignSpec()) + int(loc.hasWrittenWidthSpec()) + + int(loc.hasWrittenTypeSpec())) + == 1; + } + if (isa<TagType>(type) || isa<TemplateTypeParmType>(type) || isa<AutoType>(type) + || isa<DecltypeType>(type) || isa<TypedefType>(type)) + { + return true; + } + if (auto const t = dyn_cast<ElaboratedType>(type)) { + return t->getKeyword() == ETK_None; + } + return false; +} + class CStyleCast: - public RecursiveASTVisitor<CStyleCast>, public loplugin::Plugin + public RecursiveASTVisitor<CStyleCast>, public loplugin::RewritePlugin { public: - explicit CStyleCast(loplugin::InstantiationData const & data): Plugin(data) + explicit CStyleCast(loplugin::InstantiationData const & data): RewritePlugin(data) {} virtual void run() override { @@ -95,6 +175,12 @@ public: } } + bool TraverseInitListExpr(InitListExpr * expr, DataRecursionQueue * queue = nullptr) { + return WalkUpFromInitListExpr(expr) + && TraverseSynOrSemInitListExpr( + expr->isSemanticForm() ? expr : expr->getSemanticForm(), queue); + } + bool TraverseLinkageSpecDecl(LinkageSpecDecl * decl); bool VisitCStyleCastExpr(const CStyleCastExpr * expr); @@ -106,7 +192,17 @@ private: bool isSharedCAndCppCode(SourceLocation location) const; + bool isLastTokenOfImmediateMacroBodyExpansion( + SourceLocation loc, SourceLocation * macroEnd = nullptr) const; + + bool rewriteArithmeticCast(CStyleCastExpr const * expr, char const ** replacement); + unsigned int externCContexts_ = 0; + std::set<SourceLocation> rewritten_; + // needed when rewriting in macros, in general to avoid "double code replacement, possible + // plugin error" warnings, and in particular to avoid adding multiple sets of parens around + // sub-exprs + std::set<CStyleCastExpr const *> rewrittenSubExprs_; }; const char * recommendedFix(clang::CastKind ck) { @@ -141,13 +237,18 @@ bool CStyleCast::VisitCStyleCastExpr(const CStyleCastExpr * expr) { if( expr->getCastKind() == CK_IntegralCast ) { return true; } + if (isSharedCAndCppCode(expr->getLocStart())) { + return true; + } char const * perf = nullptr; if( expr->getCastKind() == CK_NoOp ) { if (!((expr->getSubExpr()->getType()->isPointerType() && expr->getType()->isPointerType()) || expr->getTypeAsWritten()->isReferenceType())) { - return true; + if (rewriteArithmeticCast(expr, &perf)) { + return true; + } } if (isConstCast( expr->getSubExprAsWritten()->getType(), @@ -156,9 +257,6 @@ bool CStyleCast::VisitCStyleCastExpr(const CStyleCastExpr * expr) { perf = "const_cast"; } } - if (isSharedCAndCppCode(expr->getLocStart())) { - return true; - } std::string incompFrom; std::string incompTo; if( expr->getCastKind() == CK_BitCast ) { @@ -231,7 +329,335 @@ bool CStyleCast::isSharedCAndCppCode(SourceLocation location) const { || compiler.getSourceManager().isMacroBodyExpansion(location)); } -loplugin::Plugin::Registration< CStyleCast > X("cstylecast"); +bool CStyleCast::isLastTokenOfImmediateMacroBodyExpansion( + SourceLocation loc, SourceLocation * macroEnd) const +{ + assert(compiler.getSourceManager().isMacroBodyExpansion(loc)); + auto const spell = compiler.getSourceManager().getSpellingLoc(loc); + auto name = Lexer::getImmediateMacroName( + loc, compiler.getSourceManager(), compiler.getLangOpts()); + while (name.startswith("\\\n")) { + name = name.drop_front(2); + while (!name.empty() + && (name.front() == ' ' || name.front() == '\t' || name.front() == '\n' + || name.front() == '\v' || name.front() == '\f')) + { + name = name.drop_front(1); + } + } + auto const MI + = (compiler.getPreprocessor().getMacroDefinitionAtLoc( + &compiler.getASTContext().Idents.get(name), spell) + .getMacroInfo()); + assert(MI != nullptr); + if (spell == MI->getDefinitionEndLoc()) { + if (macroEnd != nullptr) { + *macroEnd = compiler.getSourceManager().getImmediateExpansionRange(loc).second; + } + return true; + } + return false; +} + +bool CStyleCast::rewriteArithmeticCast(CStyleCastExpr const * expr, char const ** replacement) { + assert(replacement != nullptr); + auto const sub = expr->getSubExprAsWritten(); + auto const functional = isLiteralLike(sub) + && canBeUsedForFunctionalCast(expr->getTypeInfoAsWritten()); + *replacement = functional ? "functional cast" : "static_cast"; + if (rewriter == nullptr) { + return false; + } + // Doing modifications for a chain of C-style casts as in + // + // (foo)(bar)(baz)x + // + // leads to unpredictable results, so only rewrite them one at a time, starting with the + // outermost: + if (auto const e = dyn_cast<CStyleCastExpr>(sub)) { + rewrittenSubExprs_.insert(e); + } + if (rewrittenSubExprs_.find(expr) != rewrittenSubExprs_.end()) { + return false; + } + // Two or four ranges to replace: + // First is the CStyleCast's LParen, plus following whitespace, replaced with either "" or + // "static_cast<". (TOOD: insert space before "static_cast<" when converting "else(int)...".) + // Second is the CStyleCast's RParen, plus preceding and following whitespace, replaced with + // either "" or ">". + // If the sub expr is not a ParenExpr, third is the sub expr's begin, inserting "(", and fourth + // is the sub expr's end, inserting ")". + // (The reason the second and third are not combined is in case there's a comment between them.) + auto firstBegin = expr->getLParenLoc(); + auto secondBegin = expr->getRParenLoc(); + while (compiler.getSourceManager().isMacroArgExpansion(firstBegin) + && compiler.getSourceManager().isMacroArgExpansion(secondBegin) + && (compiler.getSourceManager().getImmediateExpansionRange(firstBegin) + == compiler.getSourceManager().getImmediateExpansionRange(secondBegin))) + { + firstBegin = compiler.getSourceManager().getImmediateSpellingLoc(firstBegin); + secondBegin = compiler.getSourceManager().getImmediateSpellingLoc(secondBegin); + } + if (compiler.getSourceManager().isMacroBodyExpansion(firstBegin) + && compiler.getSourceManager().isMacroBodyExpansion(secondBegin) + && (compiler.getSourceManager().getImmediateMacroCallerLoc(firstBegin) + == compiler.getSourceManager().getImmediateMacroCallerLoc(secondBegin))) + { + firstBegin = compiler.getSourceManager().getSpellingLoc(firstBegin); + secondBegin = compiler.getSourceManager().getSpellingLoc(secondBegin); + } + auto third = sub->getLocStart(); + auto fourth = sub->getLocEnd(); + bool macro = false; + // Ensure that + // + // #define FOO(x) (int)x + // FOO(y) + // + // is changed to + // + // #define FOO(x) static_cast<int>(x) + // FOO(y) + // + // instead of + // + // #define FOO(x) static_cast<int>x + // FOO((y)) + while (compiler.getSourceManager().isMacroArgExpansion(third) + && compiler.getSourceManager().isMacroArgExpansion(fourth) + && (compiler.getSourceManager().getImmediateExpansionRange(third) + == compiler.getSourceManager().getImmediateExpansionRange(fourth)) + && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(third)) + //TODO: check fourth is at end of immediate macro expansion, but + // SourceManager::isAtEndOfImmediateMacroExpansion requires a location pointing at the + // character end of the last token + { + auto const range = compiler.getSourceManager().getImmediateExpansionRange(third); + third = range.first; + fourth = range.second; + macro = true; + assert(third.isValid()); + } + while (compiler.getSourceManager().isMacroArgExpansion(third) + && compiler.getSourceManager().isMacroArgExpansion(fourth) + && (compiler.getSourceManager().getImmediateExpansionRange(third) + == compiler.getSourceManager().getImmediateExpansionRange(fourth))) + { + third = compiler.getSourceManager().getImmediateSpellingLoc(third); + fourth = compiler.getSourceManager().getImmediateSpellingLoc(fourth); + } + if (isa<ParenExpr>(sub)) { + // Ensure that with + // + // #define FOO (x) + // + // a cast like + // + // (int) FOO + // + // is changed to + // + // static_cast<int>(FOO) + // + // instead of + // + // static_cast<int>FOO + for (;; macro = true) { + if (!(compiler.getSourceManager().isMacroBodyExpansion(third) + && compiler.getSourceManager().isMacroBodyExpansion(fourth) + && (compiler.getSourceManager().getImmediateMacroCallerLoc(third) + == compiler.getSourceManager().getImmediateMacroCallerLoc(fourth)) + && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(third) + && isLastTokenOfImmediateMacroBodyExpansion(fourth))) + { + if (!macro) { + third = fourth = SourceLocation(); + } + break; + } + auto const range = compiler.getSourceManager().getImmediateExpansionRange(third); + third = range.first; + fourth = range.second; + assert(third.isValid()); + } + if (third.isValid() && compiler.getSourceManager().isMacroBodyExpansion(third) + && compiler.getSourceManager().isMacroBodyExpansion(fourth) + && (compiler.getSourceManager().getImmediateMacroCallerLoc(third) + == compiler.getSourceManager().getImmediateMacroCallerLoc(fourth))) + { + third = compiler.getSourceManager().getSpellingLoc(third); + fourth = compiler.getSourceManager().getSpellingLoc(fourth); + assert(third.isValid()); + } + } else { + // Ensure that a cast like + // + // (int)LONG_MAX + // + // (where LONG_MAX expands to __LONG_MAX__, which in turn is a built-in expanding to a value + // like 9223372036854775807L) is changed to + // + // int(LONG_MAX) + // + // instead of trying to add the parentheses to the built-in __LONG_MAX__ definition: + for (;;) { + if (!(compiler.getSourceManager().isMacroBodyExpansion(third) + && compiler.getSourceManager().isMacroBodyExpansion(fourth) + && (compiler.getSourceManager().getImmediateMacroCallerLoc(third) + == compiler.getSourceManager().getImmediateMacroCallerLoc(fourth)) + && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(third))) + // TODO: check that fourth is at end of immediate macro expansion (but + // SourceManager::isAtEndOfImmediateMacroExpansion wants a location pointing at the + // character end) + { + break; + } + auto const range = compiler.getSourceManager().getImmediateExpansionRange(third); + third = range.first; + fourth = range.second; + } + // ...and additionally asymmetrically unwind macros only at the start or end, for code like + // + // (long)ubidi_getVisualIndex(...) + // + // (in editeng/source/editeng/impedit2.cxx) where ubidi_getVisualIndex is an object-like + // macro, or + // + // #define YY_SC_TO_UI(c) ((unsigned int) (unsigned char) c) + // + // (in hwpfilter/source/lexer.cxx): + if (!fourth.isMacroID()) { + while (compiler.getSourceManager().isMacroBodyExpansion(third) + && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(third, &third)) + {} + } + if (!third.isMacroID()) { + while (compiler.getSourceManager().isMacroBodyExpansion(fourth) + && isLastTokenOfImmediateMacroBodyExpansion(fourth, &fourth)) + {} + } else if (compiler.getSourceManager().isMacroBodyExpansion(third)) { + while (compiler.getSourceManager().isMacroArgExpansion(fourth, &fourth)) {} + } + if (compiler.getSourceManager().isMacroBodyExpansion(third) + && compiler.getSourceManager().isMacroBodyExpansion(fourth) + && (compiler.getSourceManager().getImmediateMacroCallerLoc(third) + == compiler.getSourceManager().getImmediateMacroCallerLoc(fourth))) + { + third = compiler.getSourceManager().getSpellingLoc(third); + fourth = compiler.getSourceManager().getSpellingLoc(fourth); + } + assert(third.isValid()); + } + if (firstBegin.isMacroID() || secondBegin.isMacroID() || (third.isValid() && third.isMacroID()) + || (fourth.isValid() && fourth.isMacroID())) + { + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, + "TODO: cannot rewrite C-style cast in macro, needs investigation", + expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + unsigned firstLen = Lexer::MeasureTokenLength( + firstBegin, compiler.getSourceManager(), compiler.getLangOpts()); + for (auto l = firstBegin.getLocWithOffset(std::max<unsigned>(firstLen, 1));; + l = l.getLocWithOffset(1)) + { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), compiler.getLangOpts()); + if (n != 0) { + break; + } + ++firstLen; + } + unsigned secondLen = Lexer::MeasureTokenLength( + secondBegin, compiler.getSourceManager(), compiler.getLangOpts()); + for (auto l = secondBegin.getLocWithOffset(std::max<unsigned>(secondLen, 1));; + l = l.getLocWithOffset(1)) + { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), compiler.getLangOpts()); + if (n != 0) { + break; + } + ++secondLen; + } + for (;;) { + auto l = secondBegin.getLocWithOffset(-1); + auto const c = compiler.getSourceManager().getCharacterData(l)[0]; + if (c == '\n') { + if (compiler.getSourceManager().getCharacterData(l.getLocWithOffset(-1))[0] == '\\') { + break; + } + } else if (!(c == ' ' || c == '\t' || c == '\v' || c == '\f')) { + break; + } + secondBegin = l; + ++secondLen; + } + if (rewritten_.find(firstBegin) == rewritten_.end()) { + rewritten_.insert(firstBegin); + if (!replaceText(firstBegin, firstLen, functional ? "" : "static_cast<")) { + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, "TODO: cannot rewrite #1, needs investigation", + firstBegin); + report( + DiagnosticsEngine::Note, "when rewriting this C-style cast", expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + if (!replaceText(secondBegin, secondLen, functional ? "" : ">")) { + //TODO: roll back + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, "TODO: cannot rewrite #2, needs investigation", + secondBegin); + report( + DiagnosticsEngine::Note, "when rewriting this C-style cast", expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + } + if (third.isValid()) { + if (rewritten_.find(third) == rewritten_.end()) { + rewritten_.insert(third); + if (!insertTextBefore(third, "(")) { + //TODO: roll back + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, "TODO: cannot rewrite #3, needs investigation", + third); + report( + DiagnosticsEngine::Note, "when rewriting this C-style cast", + expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + if (!insertTextAfterToken(fourth, ")")) { + //TODO: roll back + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, "TODO: cannot rewrite #4, needs investigation", + third); + report( + DiagnosticsEngine::Note, "when rewriting this C-style cast", + expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + } + } + return true; +} + +loplugin::Plugin::Registration< CStyleCast > X("cstylecast", true); } diff --git a/compilerplugins/clang/test/cstylecast.cxx b/compilerplugins/clang/test/cstylecast.cxx new file mode 100644 index 000000000000..c272005650ff --- /dev/null +++ b/compilerplugins/clang/test/cstylecast.cxx @@ -0,0 +1,63 @@ +/* -*- 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/. + */ + +namespace N +{ +enum E +{ + E1 +}; + +using T = unsigned int; +} + +static const int C + = (int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + +int main() +{ + constexpr int c1 = 0; + int const c2 = 0; + int n + = (int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (signed int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n = (int)~0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (int)-0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (int)+0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (int)!0; // expected-error {{C-style cast from 'bool' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + (0 << 0); + n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + c1; + n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + c2; + n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + C; + n = (int) // expected-error {{C-style cast from 'N::E' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + N::E1; + n = (N::E) // expected-error {{C-style cast from 'N::E' to 'N::E' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + N::E1; + n = (enum // expected-error {{C-style cast from 'N::E' to 'enum N::E' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + N::E)N::E1; + n = (N::T)0; // expected-error {{C-style cast from 'int' to 'N::T' (aka 'unsigned int') (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + n = (int) // expected-error-re {{C-style cast from {{.*}} to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + sizeof(int); + n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n; + n = (int)~n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n = (int)-n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n = (int)+n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n = (int)!n; // expected-error {{C-style cast from 'bool' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}} + (0 << n); + n = (double)0; // expected-error {{C-style cast from 'int' to 'double' (performs: functional cast) (NoOp) [loplugin:cstylecast]}} + (void)n; +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx index ba6c62a78e43..d4ebd6ed0b14 100644 --- a/compilerplugins/clang/unnecessaryparen.cxx +++ b/compilerplugins/clang/unnecessaryparen.cxx @@ -59,11 +59,11 @@ Expr const * ignoreAllImplicit(Expr const * expr) { } class UnnecessaryParen: - public RecursiveASTVisitor<UnnecessaryParen>, public loplugin::Plugin + public RecursiveASTVisitor<UnnecessaryParen>, public loplugin::RewritePlugin { public: explicit UnnecessaryParen(loplugin::InstantiationData const & data): - Plugin(data) {} + RewritePlugin(data) {} virtual void run() override { @@ -103,6 +103,10 @@ private: // SwNode::dumpAsXml (sw/source/core/docnode/node.cxx): bool isPrecededBy_BAD_CAST(Expr const * expr); + bool badCombination(SourceLocation loc, int prevOffset, int nextOffset); + + bool removeParens(ParenExpr const * expr); + std::unordered_set<ParenExpr const *> handled_; }; @@ -200,10 +204,12 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) } } } else if (isa<CXXNamedCastExpr>(subExpr)) { - report( - DiagnosticsEngine::Warning, "unnecessary parentheses around cast", - parenExpr->getLocStart()) - << parenExpr->getSourceRange(); + if (!removeParens(parenExpr)) { + report( + DiagnosticsEngine::Warning, "unnecessary parentheses around cast", + parenExpr->getLocStart()) + << parenExpr->getSourceRange(); + } handled_.insert(parenExpr); } @@ -473,6 +479,92 @@ bool UnnecessaryParen::isPrecededBy_BAD_CAST(Expr const * expr) { return std::string(p1, p2 - p1).find("BAD_CAST") != std::string::npos; } +namespace { + +bool badCombinationChar(char c) { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_' + || c == '+' || c == '-' || c == '\'' || c == '"'; +} + +} + +bool UnnecessaryParen::badCombination(SourceLocation loc, int prevOffset, int nextOffset) { + //TODO: check for start/end of file; take backslash-newline line concatentation into account + auto const c1 + = compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(prevOffset))[0]; + auto const c2 + = compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(nextOffset))[0]; + // An approximation of avoiding whatever combinations that would cause two ajacent tokens to be + // lexed differently, using, for now, letters (TODO: non-ASCII ones) and digits and '_'; '+' and + // '-' (to avoid ++, etc.); '\'' and '"' (to avoid u'x' or "foo"bar, etc.): + return badCombinationChar(c1) && badCombinationChar(c2); +} + +bool UnnecessaryParen::removeParens(ParenExpr const * expr) { + if (rewriter == nullptr) { + return false; + } + auto const firstBegin = expr->getLocStart(); + auto secondBegin = expr->getLocEnd(); + if (firstBegin.isMacroID() || secondBegin.isMacroID()) { + return false; + } + unsigned firstLen = Lexer::MeasureTokenLength( + firstBegin, compiler.getSourceManager(), compiler.getLangOpts()); + for (auto l = firstBegin.getLocWithOffset(std::max<unsigned>(firstLen, 1));; + l = l.getLocWithOffset(1)) + { + unsigned n = Lexer::MeasureTokenLength( + l, compiler.getSourceManager(), compiler.getLangOpts()); + if (n != 0) { + break; + } + ++firstLen; + } + unsigned secondLen = Lexer::MeasureTokenLength( + secondBegin, compiler.getSourceManager(), compiler.getLangOpts()); + for (;;) { + auto l = secondBegin.getLocWithOffset(-1); + auto const c = compiler.getSourceManager().getCharacterData(l)[0]; + if (c == '\n') { + if (compiler.getSourceManager().getCharacterData(l.getLocWithOffset(-1))[0] == '\\') { + break; + } + } else if (!(c == ' ' || c == '\t' || c == '\v' || c == '\f')) { + break; + } + secondBegin = l; + ++secondLen; + } + if (!replaceText(firstBegin, firstLen, badCombination(firstBegin, -1, firstLen) ? " " : "")) { + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, + "TODO: cannot rewrite opening parenthesis, needs investigation", + firstBegin); + report( + DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + if (!replaceText(secondBegin, secondLen, badCombination(secondBegin, -1, secondLen) ? " " : "")) + { + //TODO: roll back first change + if (isDebugMode()) { + report( + DiagnosticsEngine::Fatal, + "TODO: cannot rewrite closing parenthesis, needs investigation", + secondBegin); + report( + DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc()) + << expr->getSourceRange(); + } + return false; + } + return true; +} + loplugin::Plugin::Registration< UnnecessaryParen > X("unnecessaryparen", true); } diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index ff3f02c3a233..29a6651cdb79 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -17,6 +17,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/constparams \ compilerplugins/clang/test/convertlong \ compilerplugins/clang/test/cppunitassertequals \ + compilerplugins/clang/test/cstylecast \ compilerplugins/clang/test/datamembershadow \ compilerplugins/clang/test/dodgyswitch \ compilerplugins/clang/test/externvar \ |