[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-29 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL closed 
https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-27 Thread Congcong Cai via cfe-commits 

https://github.com/HerrCai0907 approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-27 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL approved this pull request.

LGTM.

https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-26 Thread Bhuminjay Soni via cfe-commits 

https://github.com/11happy approved this pull request.

LGTM. 
Thank you for CCing me.

https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-26 Thread Julian Schmidt via cfe-commits 

5chmidti wrote:

CC @11happy (couldn't add you as a reviewer)

https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-26 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-tools-extra

Author: Julian Schmidt (5chmidti)


Changes

When a binary operator is the last operand of a macro, the end location
that is past the `BinaryOperator` will be inside the macro and therefore an
invalid location to insert a `FixIt` into, which is why the check bails
when encountering such a pattern.
However, the end location is only required for the `FixIt` and the
diagnostic can still be emitted, just without an attached fix.


---
Full diff: https://github.com/llvm/llvm-project/pull/90279.diff


2 Files Affected:

- (modified) 
clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp (+9-7) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
 (+22) 


``diff
diff --git 
a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
index d1e20b9074cec1..65fd296094915b 100644
--- a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
@@ -61,19 +61,21 @@ static void addParantheses(const BinaryOperator *BinOp,
 const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
 const clang::SourceLocation EndLoc =
 clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
-if (EndLoc.isInvalid())
-  return;
 
-Check->diag(StartLoc,
-"'%0' has higher precedence than '%1'; add parentheses to "
-"explicitly specify the order of operations")
+auto Diag =
+Check->diag(StartLoc,
+"'%0' has higher precedence than '%1'; add parentheses to "
+"explicitly specify the order of operations")
 << (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
   : ParentBinOp->getOpcodeStr())
 << (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
   : BinOp->getOpcodeStr())
-<< FixItHint::CreateInsertion(StartLoc, "(")
-<< FixItHint::CreateInsertion(EndLoc, ")")
 << SourceRange(StartLoc, EndLoc);
+
+if (EndLoc.isValid()) {
+  Diag << FixItHint::CreateInsertion(StartLoc, "(")
+   << FixItHint::CreateInsertion(EndLoc, ")");
+}
   }
 
   addParantheses(dyn_cast(BinOp->getLHS()->IgnoreImpCasts()),
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
index edbe2e1c37c770..a6045c079a4823 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
@@ -16,6 +16,13 @@ int bar(){
 return 4;
 }
 
+int sink(int);
+#define FUN(ARG) (sink(ARG))
+#define FUN2(ARG) sink((ARG))
+#define FUN3(ARG) sink(ARG)
+#define FUN4(ARG) sink(1 + ARG)
+#define FUN5(ARG) sink(4 * ARG)
+
 class fun{
 public:
 int A;
@@ -117,4 +124,19 @@ void f(){
 //CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than 
'-'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
 //CHECK-FIXES: int q = (1 MACRO_ADD (2 MACRO_MULTIPLY 3)) MACRO_OR ((4 
MACRO_AND 5) MACRO_XOR (6 MACRO_SUBTRACT (7 MACRO_DIVIDE 8)));
 int q = 1 MACRO_ADD 2 MACRO_MULTIPLY 3 MACRO_OR 4 MACRO_AND 5 MACRO_XOR 6 
MACRO_SUBTRACT 7 MACRO_DIVIDE 8; // No warning
+
+//CHECK-MESSAGES: :[[@LINE+1]]:21: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int r = FUN(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int s = FUN2(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int t = FUN3(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:18: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int u = FUN4(1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:13: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int v = FUN5(0 + 1);
 }

``




https://github.com/llvm/llvm-project/pull/90279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] fix false-negative for macros in `readability-math-missing-parentheses` (PR #90279)

2024-04-26 Thread Julian Schmidt via cfe-commits 

https://github.com/5chmidti created 
https://github.com/llvm/llvm-project/pull/90279

When a binary operator is the last operand of a macro, the end location
that is past the `BinaryOperator` will be inside the macro and therefore an
invalid location to insert a `FixIt` into, which is why the check bails
when encountering such a pattern.
However, the end location is only required for the `FixIt` and the
diagnostic can still be emitted, just without an attached fix.


>From 0b6b64b1ab203e037d55839e84ca31b8d0230ae6 Mon Sep 17 00:00:00 2001
From: Julian Schmidt 
Date: Fri, 26 Apr 2024 23:42:40 +0200
Subject: [PATCH] [clang-tidy] fix false-negative for macros in
 `readability-math-missing-parentheses`

When a binary operator is the last operand of a macro, the end location
that is past the `BinaryOperator` will be inside the macro and therefore an
invalid location to insert a `FixIt` into, which is why the check bails
when encountering such a pattern.
However, the end location is only required for the `FixIt` and the
diagnostic can still be emitted, just without an attached fix.
---
 .../MathMissingParenthesesCheck.cpp   | 16 --
 .../readability/math-missing-parentheses.cpp  | 22 +++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
index d1e20b9074cec1..65fd296094915b 100644
--- a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
@@ -61,19 +61,21 @@ static void addParantheses(const BinaryOperator *BinOp,
 const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
 const clang::SourceLocation EndLoc =
 clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
-if (EndLoc.isInvalid())
-  return;
 
-Check->diag(StartLoc,
-"'%0' has higher precedence than '%1'; add parentheses to "
-"explicitly specify the order of operations")
+auto Diag =
+Check->diag(StartLoc,
+"'%0' has higher precedence than '%1'; add parentheses to "
+"explicitly specify the order of operations")
 << (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
   : ParentBinOp->getOpcodeStr())
 << (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
   : BinOp->getOpcodeStr())
-<< FixItHint::CreateInsertion(StartLoc, "(")
-<< FixItHint::CreateInsertion(EndLoc, ")")
 << SourceRange(StartLoc, EndLoc);
+
+if (EndLoc.isValid()) {
+  Diag << FixItHint::CreateInsertion(StartLoc, "(")
+   << FixItHint::CreateInsertion(EndLoc, ")");
+}
   }
 
   addParantheses(dyn_cast(BinOp->getLHS()->IgnoreImpCasts()),
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
index edbe2e1c37c770..a6045c079a4823 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
@@ -16,6 +16,13 @@ int bar(){
 return 4;
 }
 
+int sink(int);
+#define FUN(ARG) (sink(ARG))
+#define FUN2(ARG) sink((ARG))
+#define FUN3(ARG) sink(ARG)
+#define FUN4(ARG) sink(1 + ARG)
+#define FUN5(ARG) sink(4 * ARG)
+
 class fun{
 public:
 int A;
@@ -117,4 +124,19 @@ void f(){
 //CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than 
'-'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
 //CHECK-FIXES: int q = (1 MACRO_ADD (2 MACRO_MULTIPLY 3)) MACRO_OR ((4 
MACRO_AND 5) MACRO_XOR (6 MACRO_SUBTRACT (7 MACRO_DIVIDE 8)));
 int q = 1 MACRO_ADD 2 MACRO_MULTIPLY 3 MACRO_OR 4 MACRO_AND 5 MACRO_XOR 6 
MACRO_SUBTRACT 7 MACRO_DIVIDE 8; // No warning
+
+//CHECK-MESSAGES: :[[@LINE+1]]:21: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int r = FUN(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int s = FUN2(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int t = FUN3(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:18: warning: '*' has higher precedence than 
'+'; add parentheses to explicitly specify the order of operations 
[readability-math-missing-parentheses]
+int u = FUN4(1 * 2);
+
+