diff options
Diffstat (limited to 'compilerplugins/clang/unnecessaryparen.cxx')
-rw-r--r-- | compilerplugins/clang/unnecessaryparen.cxx | 137 |
1 files changed, 91 insertions, 46 deletions
diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx index eb53c449cd77..d24986f0b669 100644 --- a/compilerplugins/clang/unnecessaryparen.cxx +++ b/compilerplugins/clang/unnecessaryparen.cxx @@ -20,7 +20,6 @@ #include "config_clang.h" -#include "compat.hxx" #include "plugin.hxx" /** @@ -44,7 +43,7 @@ Expr const * ignoreAllImplicit(Expr const * expr) { } } else if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) { - expr = compat::getSubExpr(e); + expr = e->getSubExpr(); } else if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) { expr = e->getSubExpr(); @@ -57,11 +56,9 @@ Expr const * ignoreAllImplicit(Expr const * expr) { expr = ce->getImplicitObjectArgument(); } } -#if CLANG_VERSION >= 80000 else if (auto const e = dyn_cast<ConstantExpr>(expr)) { expr = e->getSubExpr(); } -#endif if (expr == oldExpr) return expr; } @@ -70,7 +67,7 @@ Expr const * ignoreAllImplicit(Expr const * expr) { bool isParenWorthyOpcode(BinaryOperatorKind op) { return !(BinaryOperator::isMultiplicativeOp(op) || BinaryOperator::isAdditiveOp(op) - || compat::isPtrMemOp(op)); + || BinaryOperator::isPtrMemOp(op)); } class UnnecessaryParen: @@ -86,8 +83,6 @@ public: // fixing this, makes the source in the .y files look horrible if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/unoidl/source/sourceprovider-parser.cxx")) return false; - if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/idlc/source/parser.cxx")) - return false; return true; } virtual void run() override @@ -148,6 +143,55 @@ private: bool removeParens(ParenExpr const * expr); + // Returns 0 if not a string literal at all: + unsigned getStringLiteralTokenCount(Expr const * expr, Expr const * parenExpr) { + if (auto const e = dyn_cast<clang::StringLiteral>(expr)) { + if (parenExpr == nullptr || !isPrecededBy_BAD_CAST(parenExpr)) { + return e->getNumConcatenated(); + } + } else if (auto const e = dyn_cast<UserDefinedLiteral>(expr)) { + clang::StringLiteral const * lit = nullptr; + switch (e->getLiteralOperatorKind()) { + case UserDefinedLiteral::LOK_Template: + { + auto const decl = e->getDirectCallee(); + assert(decl != nullptr); + auto const args = decl->getTemplateSpecializationArgs(); + assert(args != nullptr); + if (args->size() == 1 && (*args)[0].getKind() == TemplateArgument::Declaration) + { + if (auto const d + = dyn_cast<TemplateParamObjectDecl>((*args)[0].getAsDecl())) + { + if (d->getValue().isStruct() || d->getValue().isUnion()) { + //TODO: There appears to be no way currently to get at the original + // clang::StringLiteral expression from which this struct/union + // non-type template argument was constructed, so no way to tell + // whether it was written as a single literal (=> in which case we + // should warn about unnecessary parentheses) or as a concatenation + // of multiple literals (=> in which case we should not warn). So + // be conservative and not warn at all (by pretending to have more + // than one token): + return 2; + } + } + } + break; + } + case UserDefinedLiteral::LOK_String: + assert(e->getNumArgs() == 2); + lit = dyn_cast<clang::StringLiteral>(e->getArg(0)->IgnoreImplicit()); + break; + default: + break; + } + if (lit != nullptr) { + return lit->getNumConcatenated(); + } + } + return 0; + } + std::unordered_set<ParenExpr const *> handled_; }; @@ -175,7 +219,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) { if (ignoreLocation(parenExpr)) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; if (handled_.find(parenExpr) != handled_.end()) return true; @@ -184,11 +228,11 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) if (auto subParenExpr = dyn_cast<ParenExpr>(subExpr)) { - if (compat::getBeginLoc(subParenExpr).isMacroID()) + if (subParenExpr->getBeginLoc().isMacroID()) return true; report( DiagnosticsEngine::Warning, "parentheses around parentheses", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(subParenExpr); } @@ -201,7 +245,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) if (!isPrecededBy_BAD_CAST(parenExpr)) { report( DiagnosticsEngine::Warning, "unnecessary parentheses around identifier", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); } @@ -210,7 +254,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) || isa<CXXBoolLiteralExpr>(subExpr) || isa<CXXNullPtrLiteralExpr>(subExpr) || isa<ObjCBoolLiteralExpr>(subExpr)) { - auto const loc = compat::getBeginLoc(subExpr); + auto const loc = subExpr->getBeginLoc(); if (loc.isMacroID() && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(loc)) { // just in case the macro could also expand to something that /would/ require @@ -219,15 +263,15 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) } report( DiagnosticsEngine::Warning, "unnecessary parentheses around literal", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); - } else if (auto const e = dyn_cast<clang::StringLiteral>(subExpr)) { - if (e->getNumConcatenated() == 1 && !isPrecededBy_BAD_CAST(parenExpr)) { + } else if (isa<clang::StringLiteral>(subExpr) || isa<UserDefinedLiteral>(subExpr)) { + if (getStringLiteralTokenCount(subExpr, parenExpr) == 1) { report( DiagnosticsEngine::Warning, "unnecessary parentheses around single-token string literal", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); } @@ -239,7 +283,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) report( DiagnosticsEngine::Warning, "unnecessary parentheses around signed numeric literal", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); } @@ -248,7 +292,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) if (!removeParens(parenExpr)) { report( DiagnosticsEngine::Warning, "unnecessary parentheses around cast", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); } handled_.insert(parenExpr); @@ -256,7 +300,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) if (isa<CXXThisExpr>(ignoreAllImplicit(memberExpr->getBase()))) { report( DiagnosticsEngine::Warning, "unnecessary parentheses around member expr", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); } @@ -267,8 +311,10 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr) bool UnnecessaryParen::VisitIfStmt(const IfStmt* ifStmt) { - handleUnreachableCodeConditionParens(ifStmt->getCond()); - VisitSomeStmt(ifStmt, ifStmt->getCond(), "if"); + if (auto const cond = ifStmt->getCond()) { + handleUnreachableCodeConditionParens(cond); + VisitSomeStmt(ifStmt, cond, "if"); + } return true; } @@ -314,7 +360,7 @@ bool UnnecessaryParen::VisitReturnStmt(const ReturnStmt* returnStmt) auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(returnStmt->getRetValue())); if (!parenExpr) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; // assignments need extra parentheses or they generate a compiler warning auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr()); @@ -323,11 +369,12 @@ bool UnnecessaryParen::VisitReturnStmt(const ReturnStmt* returnStmt) // only non-operator-calls for now auto subExpr = ignoreAllImplicit(parenExpr->getSubExpr()); - if (isa<CallExpr>(subExpr) && !isa<CXXOperatorCallExpr>(subExpr)) + if (isa<CallExpr>(subExpr) && !isa<CXXOperatorCallExpr>(subExpr) + && !isa<UserDefinedLiteral>(subExpr)) { report( DiagnosticsEngine::Warning, "parentheses immediately inside return statement", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); } @@ -344,7 +391,7 @@ void UnnecessaryParen::VisitSomeStmt(const Stmt * stmt, const Expr* cond, String if (handled_.find(parenExpr) != handled_.end()) { return; } - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return; // assignments need extra parentheses or they generate a compiler warning auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr()); @@ -357,7 +404,7 @@ void UnnecessaryParen::VisitSomeStmt(const Stmt * stmt, const Expr* cond, String } report( DiagnosticsEngine::Warning, "parentheses immediately inside %0 statement", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << stmtName << parenExpr->getSourceRange(); handled_.insert(parenExpr); @@ -381,15 +428,18 @@ bool UnnecessaryParen::VisitCallExpr(const CallExpr* callExpr) auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(callExpr->getArg(0))); if (!parenExpr) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; // assignments need extra parentheses or they generate a compiler warning auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr()); if (binaryOp && binaryOp->getOpcode() == BO_Assign) return true; + if (getStringLiteralTokenCount(parenExpr->getSubExpr()->IgnoreImplicit(), nullptr) > 1) { + return true; + } report( DiagnosticsEngine::Warning, "parentheses immediately inside single-arg call", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); return true; @@ -403,7 +453,7 @@ bool UnnecessaryParen::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr) auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(deleteExpr->getArgument())); if (!parenExpr) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; // assignments need extra parentheses or they generate a compiler warning auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr()); @@ -411,7 +461,7 @@ bool UnnecessaryParen::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr) return true; report( DiagnosticsEngine::Warning, "parentheses immediately inside delete expr", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); return true; @@ -437,18 +487,16 @@ bool UnnecessaryParen::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callE auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(callExpr->getArg(1))); if (!parenExpr) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; // Sometimes parentheses make the RHS of an assignment easier to read by // visually disambiguating the = from a call to == auto sub = parenExpr->getSubExpr(); -#if CLANG_VERSION >= 100000 if (auto const e = dyn_cast<CXXRewrittenBinaryOperator>(sub)) { if (isParenWorthyOpcode(e->getDecomposedForm().Opcode)) { return true; } } -#endif if (auto subBinOp = dyn_cast<BinaryOperator>(sub)) { if (isParenWorthyOpcode(subBinOp->getOpcode())) @@ -465,7 +513,7 @@ bool UnnecessaryParen::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callE report( DiagnosticsEngine::Warning, "parentheses immediately inside assignment", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); return true; @@ -481,17 +529,15 @@ bool UnnecessaryParen::VisitVarDecl(const VarDecl* varDecl) auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(varDecl->getInit())); if (!parenExpr) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; // Sometimes parentheses make the RHS of an assignment easier to read by // visually disambiguating the = from a call to == auto sub = parenExpr->getSubExpr(); -#if CLANG_VERSION >= 100000 if (auto const e = dyn_cast<CXXRewrittenBinaryOperator>(sub)) { sub = e->getDecomposedForm().InnerBinOp; } -#endif if (auto subBinOp = dyn_cast<BinaryOperator>(sub)) { if (!(subBinOp->isMultiplicativeOp() || subBinOp->isAdditiveOp() || subBinOp->isPtrMemOp())) @@ -514,7 +560,7 @@ bool UnnecessaryParen::VisitVarDecl(const VarDecl* varDecl) report( DiagnosticsEngine::Warning, "parentheses immediately inside vardecl statement", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); return true; @@ -530,7 +576,7 @@ bool UnnecessaryParen::VisitMemberExpr(const MemberExpr* memberExpr) return true; if (handled_.find(parenExpr) != handled_.end()) return true; - if (compat::getBeginLoc(parenExpr).isMacroID()) + if (parenExpr->getBeginLoc().isMacroID()) return true; auto sub = parenExpr->getSubExpr(); @@ -548,7 +594,7 @@ bool UnnecessaryParen::VisitMemberExpr(const MemberExpr* memberExpr) report( DiagnosticsEngine::Warning, "unnecessary parentheses around member expr", - compat::getBeginLoc(parenExpr)) + parenExpr->getBeginLoc()) << parenExpr->getSourceRange(); handled_.insert(parenExpr); return true; @@ -561,7 +607,6 @@ bool UnnecessaryParen::VisitMemberExpr(const MemberExpr* memberExpr) // in Clang's lib/Analysis/ReachableCode.cpp looks for, descending into certain unary and binary // operators): void UnnecessaryParen::handleUnreachableCodeConditionParens(Expr const * expr) { - // Cf. : auto const e = ignoreAllImplicit(expr); if (auto const e1 = dyn_cast<ParenExpr>(e)) { auto const sub = e1->getSubExpr(); @@ -583,12 +628,12 @@ void UnnecessaryParen::handleUnreachableCodeConditionParens(Expr const * expr) { } bool UnnecessaryParen::isPrecededBy_BAD_CAST(Expr const * expr) { - if (compat::getBeginLoc(expr).isMacroID()) { + if (expr->getBeginLoc().isMacroID()) { return false; } SourceManager& SM = compiler.getSourceManager(); - const char *p1 = SM.getCharacterData( compat::getBeginLoc(expr).getLocWithOffset(-10) ); - const char *p2 = SM.getCharacterData( compat::getBeginLoc(expr) ); + const char *p1 = SM.getCharacterData( expr->getBeginLoc().getLocWithOffset(-10) ); + const char *p2 = SM.getCharacterData( expr->getBeginLoc() ); return std::string(p1, p2 - p1).find("BAD_CAST") != std::string::npos; } @@ -617,8 +662,8 @@ bool UnnecessaryParen::removeParens(ParenExpr const * expr) { if (rewriter == nullptr) { return false; } - auto const firstBegin = compat::getBeginLoc(expr); - auto secondBegin = compat::getEndLoc(expr); + auto const firstBegin = expr->getBeginLoc(); + auto secondBegin = expr->getEndLoc(); if (firstBegin.isMacroID() || secondBegin.isMacroID()) { return false; } |