[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {
+
+// Skip spaces and tabs from both strings
+while (LIdx < Left->size() && (LData[LIdx] == ' ' || LData[LIdx] == '\t')) 
{
+  LIdx += 1;
+}
+while (RIdx < Right->size() &&
+   (RData[RIdx] == ' ' || RData[RIdx] == '\t')) {
+  RIdx += 1;
+}
+
+// If all not tabs and spaces are the same then return true
+if (LIdx >= Left->size() && RIdx >= Right->size())
+  return true;
+
+// If any characters differs then return false
+else if (LData[LIdx] != RData[RIdx])
+  return false;
+
+// If current char is the same ==> continue search
+else {
+  LIdx += 1;
+  RIdx += 1;
+}
+  }
+  // Shortest string is processed: issue a verdict
+  return LIdx >= Left->size() && RIdx >= Right->size();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+   const ASTContext *Context) {
+
+  if (!BinOp)
+return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {

earnol wrote:

Checking expressions like +VALUE was not a goal of this function in the first 
place.
But i agree the case like `if(+VALUE == +VALUE)` is redundant and should be 
properly detected. I'll add it to lit tests and make sure it will get detected.

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread via cfe-commits 

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {

earnol wrote:

I see. StringRef drop_front is calling substr version 
(https://llvm.org/doxygen/SimplifyLibCalls_8cpp_source.html#l00356) not doing 
string duplication, so this approach should have the same the same efficiency.

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -1250,6 +1250,9 @@ Improvements
 - Improved the handling of the ``ownership_returns`` attribute. Now, Clang 
reports an
   error if the attribute is attached to a function that returns a non-pointer 
value.
   Fixes (#GH99501)
+- Improved ``misc-redundant-expression`` checker potentially reducing number 
of false

PiotrZSL wrote:

Wrong file, remove this, you got already entry in other ReleaseNotes

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,

PiotrZSL wrote:

ptr to llvm::StringRef  is a bad habit, pass it by value

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {
+
+// Skip spaces and tabs from both strings
+while (LIdx < Left->size() && (LData[LIdx] == ' ' || LData[LIdx] == '\t')) 
{
+  LIdx += 1;
+}
+while (RIdx < Right->size() &&
+   (RData[RIdx] == ' ' || RData[RIdx] == '\t')) {
+  RIdx += 1;
+}
+
+// If all not tabs and spaces are the same then return true
+if (LIdx >= Left->size() && RIdx >= Right->size())
+  return true;
+
+// If any characters differs then return false
+else if (LData[LIdx] != RData[RIdx])
+  return false;
+
+// If current char is the same ==> continue search
+else {
+  LIdx += 1;
+  RIdx += 1;
+}
+  }
+  // Shortest string is processed: issue a verdict
+  return LIdx >= Left->size() && RIdx >= Right->size();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+   const ASTContext *Context) {
+
+  if (!BinOp)
+return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {
+// Left is macro so right macro too
+if (Rsr.getBegin().isMacroID()) {
+  // Both sides are macros so they are same macro or literal
+  const llvm::StringRef L = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Lsr), SM, LangOptions(), 0);
+  const llvm::StringRef R = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Rsr), SM, LangOptions(), 0);
+  return areStringsSameIgnoreSpaces(&L, &R);
+}
+// Left is macro but right is not so they are not same macro or literal
+return false;
+  } else {
+const auto *Lil = dyn_cast(Lhs);
+const auto *Ril = dyn_cast(Rhs);
+if (Lil && Ril) {

PiotrZSL wrote:

drop {}

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {
+
+// Skip spaces and tabs from both strings
+while (LIdx < Left->size() && (LData[LIdx] == ' ' || LData[LIdx] == '\t')) 
{
+  LIdx += 1;
+}
+while (RIdx < Right->size() &&
+   (RData[RIdx] == ' ' || RData[RIdx] == '\t')) {
+  RIdx += 1;
+}
+
+// If all not tabs and spaces are the same then return true
+if (LIdx >= Left->size() && RIdx >= Right->size())
+  return true;
+
+// If any characters differs then return false
+else if (LData[LIdx] != RData[RIdx])
+  return false;
+
+// If current char is the same ==> continue search
+else {
+  LIdx += 1;
+  RIdx += 1;
+}
+  }
+  // Shortest string is processed: issue a verdict
+  return LIdx >= Left->size() && RIdx >= Right->size();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+   const ASTContext *Context) {
+
+  if (!BinOp)
+return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {
+// Left is macro so right macro too
+if (Rsr.getBegin().isMacroID()) {
+  // Both sides are macros so they are same macro or literal
+  const llvm::StringRef L = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Lsr), SM, LangOptions(), 0);
+  const llvm::StringRef R = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Rsr), SM, LangOptions(), 0);
+  return areStringsSameIgnoreSpaces(&L, &R);
+}
+// Left is macro but right is not so they are not same macro or literal
+return false;
+  } else {

PiotrZSL wrote:

no need for else, previous if ends with return.

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {
+
+// Skip spaces and tabs from both strings
+while (LIdx < Left->size() && (LData[LIdx] == ' ' || LData[LIdx] == '\t')) 
{
+  LIdx += 1;
+}
+while (RIdx < Right->size() &&
+   (RData[RIdx] == ' ' || RData[RIdx] == '\t')) {
+  RIdx += 1;
+}
+
+// If all not tabs and spaces are the same then return true
+if (LIdx >= Left->size() && RIdx >= Right->size())
+  return true;
+
+// If any characters differs then return false
+else if (LData[LIdx] != RData[RIdx])
+  return false;
+
+// If current char is the same ==> continue search
+else {
+  LIdx += 1;
+  RIdx += 1;
+}
+  }
+  // Shortest string is processed: issue a verdict
+  return LIdx >= Left->size() && RIdx >= Right->size();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+   const ASTContext *Context) {
+
+  if (!BinOp)
+return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {
+// Left is macro so right macro too
+if (Rsr.getBegin().isMacroID()) {
+  // Both sides are macros so they are same macro or literal
+  const llvm::StringRef L = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Lsr), SM, LangOptions(), 0);
+  const llvm::StringRef R = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Rsr), SM, LangOptions(), 0);

PiotrZSL wrote:

get proper LangOptions from ASTContext

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {
+
+// Skip spaces and tabs from both strings
+while (LIdx < Left->size() && (LData[LIdx] == ' ' || LData[LIdx] == '\t')) 
{
+  LIdx += 1;
+}
+while (RIdx < Right->size() &&
+   (RData[RIdx] == ' ' || RData[RIdx] == '\t')) {
+  RIdx += 1;
+}
+
+// If all not tabs and spaces are the same then return true
+if (LIdx >= Left->size() && RIdx >= Right->size())
+  return true;
+
+// If any characters differs then return false
+else if (LData[LIdx] != RData[RIdx])
+  return false;
+
+// If current char is the same ==> continue search
+else {
+  LIdx += 1;
+  RIdx += 1;
+}
+  }
+  // Shortest string is processed: issue a verdict
+  return LIdx >= Left->size() && RIdx >= Right->size();
+}
+
+static bool areExprsSameMacroOrLiteral(const BinaryOperator *BinOp,
+   const ASTContext *Context) {
+
+  if (!BinOp)
+return false;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+  const SourceManager &SM = Context->getSourceManager();
+
+  const SourceRange Lsr = Lhs->getSourceRange();
+  const SourceRange Rsr = Rhs->getSourceRange();
+  if (Lsr.getBegin().isMacroID()) {

PiotrZSL wrote:

problem with this is that you do not know if every part of LHS or RHS is an 
macro, because you checking only source of first token. If I would write: 
+VALUE, where #define VALUE 10, then it would fail.

I think that there is some function in /utils/ that may help here

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {
+return true;
+  }
+  // Do running index comparison
+  size_t LIdx = 0;
+  size_t RIdx = 0;
+  const char *LData = Left->data();
+  const char *RData = Right->data();
+  while (LIdx < Left->size() && RIdx < Right->size()) {

PiotrZSL wrote:

write this using ltrim and drop_front, we in 2025, no need to go back to legacy 
C++

```
  Left = Left.trim();
  Right = Right.trim();
  while(!Left.empty() && !Right.empty()) {
 Left = Left.ltrim();
 Right = Right.ltrim();
 if (Left.front() != Right.front()) return false;
Left = Left.drop_front();
Right = Right.drop_front();
  }
  return Left.empty() && Right.empty();
```

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread Piotr Zegar via cfe-commits 


@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  const SourceLocation RhsLoc = RhsExpr->getExprLoc();
 
   return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
+
+static bool areStringsSameIgnoreSpaces(const llvm::StringRef *Left,
+   const llvm::StringRef *Right) {
+  if (Left == Right)
+return true;
+  if (Left->compare(*Right) == 0) {

PiotrZSL wrote:

no need for "{}", use operator == (it's a free standing function, that's why 
you may not see it in documentation)

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-20 Thread via cfe-commits 

earnol wrote:

A gentle reminder to request a review for this PR since it is a week without a 
review.

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

https://github.com/earnol updated 
https://github.com/llvm/llvm-project/pull/122841

>From 9f658b50f70b1363d2daa26864f59934718384cc Mon Sep 17 00:00:00 2001
From: Vladislav Aranov 
Date: Mon, 13 Jan 2025 20:19:58 -0500
Subject: [PATCH] [clang-tidy] Address false positives in
 misc-redundant-expression checker

Address situations when misc-redundant-expression checker provides excessive
diagnostics for situations with different macros having the same value.
In perticular situations described in the initial report of
https://github.com/llvm/llvm-project/issues/118885 are addressed.
The situations which are popped inside discussion like
if (A + B == B + A) for macros are not properly addressed by this patch.
But I still believe even in the current state the patch can be useful.
---
 .../misc/RedundantExpressionCheck.cpp | 166 ---
 clang-tools-extra/docs/ReleaseNotes.rst   |   2 +-
 .../checks/misc/redundant-expression.rst  |  14 +-
 .../checkers/misc/redundant-expression.cpp| 193 ++
 clang/docs/ReleaseNotes.rst   |   3 +
 5 files changed, 347 insertions(+), 31 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index fc35bc22c52e04..31650309ecf8b3 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -139,10 +139,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr 
*Right) {
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
-const auto *LeftUnaryExpr =
-cast(Left);
-const auto *RightUnaryExpr =
-cast(Right);
+const auto *LeftUnaryExpr = cast(Left);
+const auto *RightUnaryExpr = cast(Right);
 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
   return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
  LeftUnaryExpr->getArgumentType() ==
@@ -699,7 +697,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 
 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
 OperandExpr = OverloadedOperatorExpr;
-Opcode = 
BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+Opcode = BinaryOperator::getOverloadedOpcode(
+OverloadedOperatorExpr->getOperator());
 
 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
   return false;
@@ -728,7 +727,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const 
ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+   const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast(BinOp->getRHS());
 
@@ -747,6 +747,28 @@ static bool areSidesBinaryConstExpressions(const 
BinaryOperator *&BinOp, const A
   return false;
 }
 
+static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  if (areSidesBinaryConstExpressions(BinOp, AstCtx))
+return true;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+
+  if (!Lhs || !Rhs)
+return false;
+
+  auto IsDefineExpr = [AstCtx](const Expr *E) {
+const SourceRange Lsr = E->getSourceRange();
+if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
+!E->isIntegerConstantExpr(*AstCtx))
+  return false;
+return true;
+  };
+
+  return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
+}
+
 // Retrieves integer constant subexpressions from binary operator expressions
 // that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
@@ -785,7 +807,7 @@ static bool retrieveConstExprFromBothSides(const 
BinaryOperator *&BinOp,
 }
 
 static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
-const SourceManager &SM) {
+ const SourceManager &SM) {
   if (T1.getKind() != T2.getKind())
 return false;
   if (T1.isNot(tok::raw_identifier))
@@ -808,8 +830,8 @@ static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-  SourceRange Lsr = LhsExpr->getSourceRange();
-  SourceRange Rsr = RhsExpr->getSourceRange();
+  const SourceRange Lsr = LhsExpr->getSourceRange();
+  const SourceRange Rsr = RhsExpr->getSourceRange();
   if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
 
@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

https://github.com/earnol updated 
https://github.com/llvm/llvm-project/pull/122841

>From 1e0f301f1c41e7f678c3eac2a067a7c81caa3228 Mon Sep 17 00:00:00 2001
From: Vladislav Aranov 
Date: Mon, 13 Jan 2025 20:19:58 -0500
Subject: [PATCH] [clang-tidy] Address false positives in
 misc-redundant-expression checker

Address situations when misc-redundant-expression checker provides excessive
diagnostics for situations with different macros having the same value.
In perticular situations described in the initial report of
https://github.com/llvm/llvm-project/issues/118885 are addressed.
The situations which are popped inside discussion like
if (A + B == B + A) for macros are not properly addressed by this patch.
But I still believe even in the current state the patch can be useful.
---
 .../misc/RedundantExpressionCheck.cpp | 166 ---
 clang-tools-extra/docs/ReleaseNotes.rst   |   2 +-
 .../checkers/misc/redundant-expression.cpp| 193 ++
 clang/docs/ReleaseNotes.rst   |   3 +
 4 files changed, 339 insertions(+), 25 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index fc35bc22c52e04..31650309ecf8b3 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -139,10 +139,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr 
*Right) {
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
-const auto *LeftUnaryExpr =
-cast(Left);
-const auto *RightUnaryExpr =
-cast(Right);
+const auto *LeftUnaryExpr = cast(Left);
+const auto *RightUnaryExpr = cast(Right);
 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
   return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
  LeftUnaryExpr->getArgumentType() ==
@@ -699,7 +697,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 
 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
 OperandExpr = OverloadedOperatorExpr;
-Opcode = 
BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+Opcode = BinaryOperator::getOverloadedOpcode(
+OverloadedOperatorExpr->getOperator());
 
 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
   return false;
@@ -728,7 +727,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const 
ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+   const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast(BinOp->getRHS());
 
@@ -747,6 +747,28 @@ static bool areSidesBinaryConstExpressions(const 
BinaryOperator *&BinOp, const A
   return false;
 }
 
+static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  if (areSidesBinaryConstExpressions(BinOp, AstCtx))
+return true;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+
+  if (!Lhs || !Rhs)
+return false;
+
+  auto IsDefineExpr = [AstCtx](const Expr *E) {
+const SourceRange Lsr = E->getSourceRange();
+if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
+!E->isIntegerConstantExpr(*AstCtx))
+  return false;
+return true;
+  };
+
+  return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
+}
+
 // Retrieves integer constant subexpressions from binary operator expressions
 // that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
@@ -785,7 +807,7 @@ static bool retrieveConstExprFromBothSides(const 
BinaryOperator *&BinOp,
 }
 
 static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
-const SourceManager &SM) {
+ const SourceManager &SM) {
   if (T1.getKind() != T2.getKind())
 return false;
   if (T1.isNot(tok::raw_identifier))
@@ -808,8 +830,8 @@ static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-  SourceRange Lsr = LhsExpr->getSourceRange();
-  SourceRange Rsr = RhsExpr->getSourceRange();
+  const SourceRange Lsr = LhsExpr->getSourceRange();
+  const SourceRange Rsr = RhsExpr->getSourceRange();
   if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
 
@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  Source

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

https://github.com/earnol updated 
https://github.com/llvm/llvm-project/pull/122841

>From 4d49ec7e0d0b1f3f38fa3c3df30dc38627d8997d Mon Sep 17 00:00:00 2001
From: Vladislav Aranov 
Date: Mon, 13 Jan 2025 20:19:58 -0500
Subject: [PATCH] [clang-tidy] Address false positives in
 misc-redundant-expression checker

Address situations when misc-redundant-expression checker provides excessive
diagnostics for situations with different macros having the same value.
In perticular situations described in the initial report of
https://github.com/llvm/llvm-project/issues/118885 are addressed.
The situations which are popped inside discussion like
if (A + B == B + A) for macros are not properly addressed by this patch.
But I still believe even in the current state the patch can be useful.
---
 .../misc/RedundantExpressionCheck.cpp | 166 ---
 .../checkers/misc/redundant-expression.cpp| 193 ++
 clang/docs/ReleaseNotes.rst   |   3 +
 3 files changed, 338 insertions(+), 24 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index fc35bc22c52e04..31650309ecf8b3 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -139,10 +139,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr 
*Right) {
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
-const auto *LeftUnaryExpr =
-cast(Left);
-const auto *RightUnaryExpr =
-cast(Right);
+const auto *LeftUnaryExpr = cast(Left);
+const auto *RightUnaryExpr = cast(Right);
 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
   return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
  LeftUnaryExpr->getArgumentType() ==
@@ -699,7 +697,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 
 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
 OperandExpr = OverloadedOperatorExpr;
-Opcode = 
BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+Opcode = BinaryOperator::getOverloadedOpcode(
+OverloadedOperatorExpr->getOperator());
 
 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
   return false;
@@ -728,7 +727,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const 
ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+   const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast(BinOp->getRHS());
 
@@ -747,6 +747,28 @@ static bool areSidesBinaryConstExpressions(const 
BinaryOperator *&BinOp, const A
   return false;
 }
 
+static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  if (areSidesBinaryConstExpressions(BinOp, AstCtx))
+return true;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+
+  if (!Lhs || !Rhs)
+return false;
+
+  auto IsDefineExpr = [AstCtx](const Expr *E) {
+const SourceRange Lsr = E->getSourceRange();
+if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
+!E->isIntegerConstantExpr(*AstCtx))
+  return false;
+return true;
+  };
+
+  return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
+}
+
 // Retrieves integer constant subexpressions from binary operator expressions
 // that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
@@ -785,7 +807,7 @@ static bool retrieveConstExprFromBothSides(const 
BinaryOperator *&BinOp,
 }
 
 static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
-const SourceManager &SM) {
+ const SourceManager &SM) {
   if (T1.getKind() != T2.getKind())
 return false;
   if (T1.isNot(tok::raw_identifier))
@@ -808,8 +830,8 @@ static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-  SourceRange Lsr = LhsExpr->getSourceRange();
-  SourceRange Rsr = RhsExpr->getSourceRange();
+  const SourceRange Lsr = LhsExpr->getSourceRange();
+  const SourceRange Rsr = RhsExpr->getSourceRange();
   if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
 
@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+  const Source

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

https://github.com/earnol updated 
https://github.com/llvm/llvm-project/pull/122841

>From abbc1142211abe99ae36ab0bde5803e375a2 Mon Sep 17 00:00:00 2001
From: Vladislav Aranov 
Date: Mon, 13 Jan 2025 20:19:58 -0500
Subject: [PATCH] [clang-tidy] Address false positives in
 misc-redundant-expression checker

Address situations when misc-redundant-expression checker provides excessive
diagnostics for situations with different macros having the same value.
In perticular situations described in the initial report of
https://github.com/llvm/llvm-project/issues/118885 are addressed.
The situations which are popped inside discussion like
if (A + B == B + A) for macros are not properly addressed by this patch.
But I still believe even in the current state the patch can be useful.
---
 .../misc/RedundantExpressionCheck.cpp | 166 ---
 clang-tools-extra/docs/ReleaseNotes.rst   |   2 +-
 .../checkers/misc/redundant-expression.cpp| 193 ++
 clang/docs/ReleaseNotes.rst   |   3 +
 4 files changed, 339 insertions(+), 25 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index fc35bc22c52e04..31650309ecf8b3 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -139,10 +139,8 @@ static bool areEquivalentExpr(const Expr *Left, const Expr 
*Right) {
 return cast(Left)->getOpcode() ==
cast(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
-const auto *LeftUnaryExpr =
-cast(Left);
-const auto *RightUnaryExpr =
-cast(Right);
+const auto *LeftUnaryExpr = cast(Left);
+const auto *RightUnaryExpr = cast(Right);
 if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
   return LeftUnaryExpr->getKind() == RightUnaryExpr->getKind() &&
  LeftUnaryExpr->getArgumentType() ==
@@ -699,7 +697,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 
 Symbol = OverloadedOperatorExpr->getArg(IntegerConstantIsFirstArg ? 1 : 0);
 OperandExpr = OverloadedOperatorExpr;
-Opcode = 
BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+Opcode = BinaryOperator::getOverloadedOpcode(
+OverloadedOperatorExpr->getOperator());
 
 if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
   return false;
@@ -728,7 +727,8 @@ static bool retrieveRelationalIntegerConstantExpr(
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const 
ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+   const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast(BinOp->getRHS());
 
@@ -747,6 +747,28 @@ static bool areSidesBinaryConstExpressions(const 
BinaryOperator *&BinOp, const A
   return false;
 }
 
+static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
+const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  if (areSidesBinaryConstExpressions(BinOp, AstCtx))
+return true;
+
+  const Expr *Lhs = BinOp->getLHS();
+  const Expr *Rhs = BinOp->getRHS();
+
+  if (!Lhs || !Rhs)
+return false;
+
+  auto IsDefineExpr = [AstCtx](const Expr *E) {
+const SourceRange Lsr = E->getSourceRange();
+if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
+!E->isIntegerConstantExpr(*AstCtx))
+  return false;
+return true;
+  };
+
+  return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
+}
+
 // Retrieves integer constant subexpressions from binary operator expressions
 // that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
@@ -785,7 +807,7 @@ static bool retrieveConstExprFromBothSides(const 
BinaryOperator *&BinOp,
 }
 
 static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
-const SourceManager &SM) {
+ const SourceManager &SM) {
   if (T1.getKind() != T2.getKind())
 return false;
   if (T1.isNot(tok::raw_identifier))
@@ -808,8 +830,8 @@ static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-  SourceRange Lsr = LhsExpr->getSourceRange();
-  SourceRange Rsr = RhsExpr->getSourceRange();
+  const SourceRange Lsr = LhsExpr->getSourceRange();
+  const SourceRange Rsr = RhsExpr->getSourceRange();
   if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
 
@@ -847,11 +869,104 @@ static bool areExprsMacroAndNonMacro(const Expr 
*&LhsExpr,
   if (!LhsExpr || !RhsExpr)
 return false;
 
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  Source

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

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

[clang] [clang-tools-extra] [clang-tidy] Address false positives in misc-redundant-expression checker (PR #122841)

2025-01-14 Thread via cfe-commits 

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