loplugin:unnecessaryparen extend to delete statements

Change-Id: Ic4383ea948876a26f791f0e5b0110cef978a26e1
Reviewed-on: https://gerrit.libreoffice.org/48027
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 26 insertions, 0 deletions

diff --git a/compilerplugins/clang/test/unnecessaryparen.cxx b/compilerplugins/clang/test/unnecessaryparen.cxx
index b7117a126edb..8621fe9e8746 100644
--- a/compilerplugins/clang/test/unnecessaryparen.cxx
+++ b/compilerplugins/clang/test/unnecessaryparen.cxx
@@ -91,6 +91,9 @@ int main()
     // "throw(x);"); it is unlikely that there are any actual occurrences of code like "-(-1)" that
     // would benefit from the parentheses readability-wise, compared to "- -1":
     (void) -(-1); // expected-error {{unnecessary parentheses around signed numeric literal [loplugin:unnecessaryparen]}}
+
+    char *p = nullptr;
+    delete (p); // expected-error {{parentheses immediately inside delete expr [loplugin:unnecessaryparen]}}
 };
 
 struct S2 {
diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx
index 8e038319fda8..caeadcac0072 100644
--- a/compilerplugins/clang/unnecessaryparen.cxx
+++ b/compilerplugins/clang/unnecessaryparen.cxx
@@ -93,6 +93,7 @@ public:
     bool VisitConditionalOperator(ConditionalOperator const * expr);
     bool VisitBinaryConditionalOperator(BinaryConditionalOperator const * expr);
     bool VisitMemberExpr(const MemberExpr *f);
+    bool VisitCXXDeleteExpr(const CXXDeleteExpr *);
 private:
     void VisitSomeStmt(Stmt const * stmt, const Expr* cond, StringRef stmtName);
 
@@ -339,6 +340,28 @@ bool UnnecessaryParen::VisitCallExpr(const CallExpr* callExpr)
     return true;
 }
 
+bool UnnecessaryParen::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr)
+{
+    if (ignoreLocation(deleteExpr))
+        return true;
+
+    auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(deleteExpr->getArgument()));
+    if (!parenExpr)
+        return true;
+    if (parenExpr->getLocStart().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;
+    report(
+        DiagnosticsEngine::Warning, "parentheses immediately inside delete expr",
+        parenExpr->getLocStart())
+        << parenExpr->getSourceRange();
+    handled_.insert(parenExpr);
+    return true;
+}
+
 bool UnnecessaryParen::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr)
 {
     if (ignoreLocation(callExpr))