summaryrefslogtreecommitdiffstats
path: root/compilerplugins/clang/unnecessaryparen.cxx
diff options
context:
space:
mode:
Diffstat (limited to 'compilerplugins/clang/unnecessaryparen.cxx')
-rw-r--r--compilerplugins/clang/unnecessaryparen.cxx137
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;
}