[clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes #74891

PiotrZSL · 2023-12-08T22:09:03Z

Check now more properly add missing parentheses to code like this: 'bool bar = true ? 1 : 0 != 0;'.

Closes #71867

…version fixes Check now more properly add missing parentheses to code like this: 'bool bar = true ? 1 : 0 != 0;'.

llvmbot · 2023-12-08T22:09:20Z

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Check now more properly add missing parentheses to code like this: 'bool bar = true ? 1 : 0 != 0;'.

Closes #71867

Patch is 31.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74891.diff

7 Files Affected:

(modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp (+6-29)
(modified) clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp (+34)
(modified) clang-tools-extra/clang-tidy/utils/FixItHintUtils.h (+4)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+1-1)
(modified) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp (+3-3)
(modified) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp (+6-6)
(modified) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp (+85-69)

diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index f0fca30de3b3c..e011598cc7c9f 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ImplicitBoolConversionCheck.h"
+#include "../utils/FixItHintUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -61,31 +62,6 @@ bool isUnaryLogicalNotOperator(const Stmt *Statement) {
   return UnaryOperatorExpr && UnaryOperatorExpr->getOpcode() == UO_LNot;
 }
 
-bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) {
-  switch (OperatorKind) {
-  case OO_New:
-  case OO_Delete: // Fall-through on purpose.
-  case OO_Array_New:
-  case OO_Array_Delete:
-  case OO_ArrowStar:
-  case OO_Arrow:
-  case OO_Call:
-  case OO_Subscript:
-    return false;
-
-  default:
-    return true;
-  }
-}
-
-bool areParensNeededForStatement(const Stmt *Statement) {
-  if (const auto *OperatorCall = dyn_cast<CXXOperatorCallExpr>(Statement)) {
-    return areParensNeededForOverloadedOperator(OperatorCall->getOperator());
-  }
-
-  return isa<BinaryOperator>(Statement) || isa<UnaryOperator>(Statement);
-}
-
 void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
                               const ImplicitCastExpr *Cast, const Stmt *Parent,
                               ASTContext &Context) {
@@ -105,9 +81,10 @@ void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
 
   const Expr *SubExpr = Cast->getSubExpr();
 
-  bool NeedInnerParens = areParensNeededForStatement(SubExpr);
+  bool NeedInnerParens =
+      SubExpr != nullptr && utils::fixit::areParensNeededForStatement(*SubExpr);
   bool NeedOuterParens =
-      Parent != nullptr && areParensNeededForStatement(Parent);
+      Parent != nullptr && utils::fixit::areParensNeededForStatement(*Parent);
 
   std::string StartLocInsertion;
 
@@ -365,7 +342,7 @@ void ImplicitBoolConversionCheck::handleCastToBool(const ImplicitCastExpr *Cast,
     return;
   }
 
-  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion %0 -> bool")
+  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion %0 -> 'bool'")
               << Cast->getSubExpr()->getType();
 
   StringRef EquivalentLiteral =
@@ -382,7 +359,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool(
     ASTContext &Context) {
   QualType DestType =
       NextImplicitCast ? NextImplicitCast->getType() : Cast->getType();
-  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion bool -> %0")
+  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion 'bool' -> %0")
               << DestType;
 
   if (const auto *BoolLiteral =
diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
index 226dd60b5bf5f..bbdd4326b0bac 100644
--- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -224,6 +224,40 @@ std::optional<FixItHint> addQualifierToVarDecl(const VarDecl &Var,
   return std::nullopt;
 }
 
+bool areParensNeededForStatement(const Stmt &Node) {
+  if (isa<ParenExpr>(&Node))
+    return false;
+
+  if (isa<clang::BinaryOperator>(&Node) || isa<UnaryOperator>(&Node))
+    return true;
+
+  if (isa<clang::ConditionalOperator>(&Node) ||
+      isa<BinaryConditionalOperator>(&Node))
+    return true;
+
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(&Node)) {
+    switch (Op->getOperator()) {
+    case OO_PlusPlus:
+      [[fallthrough]];
+    case OO_MinusMinus:
+      return Op->getNumArgs() != 2;
+    case OO_Call:
+      [[fallthrough]];
+    case OO_Subscript:
+      [[fallthrough]];
+    case OO_Arrow:
+      return false;
+    default:
+      return true;
+    };
+  }
+
+  if (isa<CStyleCastExpr>(&Node))
+    return true;
+
+  return false;
+}
+
 // Return true if expr needs to be put in parens when it is an argument of a
 // prefix unary operator, e.g. when it is a binary or ternary operator
 // syntactically.
diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
index 15894c4bf5cf8..d25bea2c6d297 100644
--- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -47,6 +47,10 @@ addQualifierToVarDecl(const VarDecl &Var, const ASTContext &Context,
 
 // \brief Format a pointer to an expression
 std::string formatDereference(const Expr &ExprNode, const ASTContext &Context);
+
+// \brief Checks whatever a expression require extra () to be always used in
+// safe way in any other expression.
+bool areParensNeededForStatement(const Stmt &Node);
 } // namespace clang::tidy::utils::fixit
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d5f49dc06254..15516fffdeebf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -423,7 +423,7 @@ Changes in existing checks
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to take
   do-while loops into account for the `AllowIntegerConditions` and
   `AllowPointerConditions` options. It also now provides more consistent
-  suggestions when parentheses are added to the return value.
+  suggestions when parentheses are added to the return value or expressions.
 
 - Improved :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability/non-const-parameter>` check to ignore
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp
index e393e297140cb..64dd3c1e051c5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp
@@ -18,7 +18,7 @@ struct Struct {
 void regularImplicitConversionIntegerToBoolIsNotIgnored() {
   int integer = 0;
   functionTaking<bool>(integer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]
   // CHECK-FIXES: functionTaking<bool>(integer != 0);
 }
 
@@ -51,12 +51,12 @@ void implicitConversionIntegerToBoolInConditionalsIsAllowed() {
 void regularImplicitConversionPointerToBoolIsNotIgnored() {
   int* pointer = nullptr;
   functionTaking<bool>(pointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(pointer != nullptr);
 
   int Struct::* memberPointer = &Struct::member;
   functionTaking<bool>(memberPointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(memberPointer != nullptr);
 }
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp
index 861677ac6c303..e4c47b06d8f84 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp
@@ -15,31 +15,31 @@ struct Struct {
 void useOldNullMacroInReplacements() {
   int* pointer = NULL;
   functionTaking<bool>(pointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool [readability-implicit-bool-conversion]
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> 'bool' [readability-implicit-bool-conversion]
   // CHECK-FIXES: functionTaking<bool>(pointer != 0);
 
   int Struct::* memberPointer = NULL;
   functionTaking<bool>(!memberPointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'int Struct::*' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'int Struct::*' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(memberPointer == 0);
 }
 
 void fixFalseLiteralConvertingToNullPointer() {
   functionTaking<int*>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int *'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'int *'
   // CHECK-FIXES: functionTaking<int*>(0);
 
   int* pointer = NULL;
   if (pointer == false) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit conversion bool -> 'int *'
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit conversion 'bool' -> 'int *'
   // CHECK-FIXES: if (pointer == 0) {}
 
   functionTaking<int Struct::*>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'int Struct::*'
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'bool' -> 'int Struct::*'
   // CHECK-FIXES: functionTaking<int Struct::*>(0);
 
   int Struct::* memberPointer = NULL;
   if (memberPointer != false) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int Struct::*'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'int Struct::*'
   // CHECK-FIXES: if (memberPointer != 0) {}
 }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
index f7f5d506a9ce0..6f1f297297188 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
@@ -21,30 +21,30 @@ void implicitConversionFromBoolSimpleCases() {
   functionTaking<bool>(boolean);
 
   functionTaking<int>(boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion bool -> 'int' [readability-implicit-bool-conversion]
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion]
   // CHECK-FIXES: functionTaking<int>(static_cast<int>(boolean));
 
   functionTaking<unsigned long>(boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'unsigned long'
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'bool' -> 'unsigned long'
   // CHECK-FIXES: functionTaking<unsigned long>(static_cast<unsigned long>(boolean));
 
   functionTaking<char>(boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'char'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'char'
   // CHECK-FIXES: functionTaking<char>(static_cast<char>(boolean));
 
   functionTaking<float>(boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion bool -> 'float'
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'bool' -> 'float'
   // CHECK-FIXES: functionTaking<float>(static_cast<float>(boolean));
 
   functionTaking<double>(boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion bool -> 'double'
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion 'bool' -> 'double'
   // CHECK-FIXES: functionTaking<double>(static_cast<double>(boolean));
 }
 
 float implicitConversionFromBoolInReturnValue() {
   bool boolean = false;
   return boolean;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion bool -> 'float'
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'bool' -> 'float'
   // CHECK-FIXES: return static_cast<float>(boolean);
 }
 
@@ -58,15 +58,15 @@ void implicitConversionFromBoolInSingleBoolExpressions(bool b1, bool b2) {
   boolean = b2 != false;
 
   int integer = boolean - 3;
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion bool -> 'int'
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion 'bool' -> 'int'
   // CHECK-FIXES: int integer = static_cast<int>(boolean) - 3;
 
   float floating = boolean / 0.3f;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'float'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'bool' -> 'float'
   // CHECK-FIXES: float floating = static_cast<float>(boolean) / 0.3f;
 
   char character = boolean;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'char'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'bool' -> 'char'
   // CHECK-FIXES: char character = static_cast<char>(boolean);
 }
 
@@ -75,41 +75,41 @@ void implicitConversionFromBoollInComplexBoolExpressions() {
   bool anotherBoolean = false;
 
   int integer = boolean && anotherBoolean;
-  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion bool -> 'int'
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion 'bool' -> 'int'
   // CHECK-FIXES: int integer = static_cast<int>(boolean && anotherBoolean);
 
   unsigned long unsignedLong = (! boolean) + 4ul;
-  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion bool -> 'unsigned long'
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'bool' -> 'unsigned long'
   // CHECK-FIXES: unsigned long unsignedLong = static_cast<unsigned long>(! boolean) + 4ul;
 
   float floating = (boolean || anotherBoolean) * 0.3f;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'float'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'bool' -> 'float'
   // CHECK-FIXES: float floating = static_cast<float>(boolean || anotherBoolean) * 0.3f;
 
   double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3;
-  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion bool -> 'double'
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion 'bool' -> 'double'
   // CHECK-FIXES: double doubleFloating = static_cast<double>(boolean && (anotherBoolean || boolean)) * 0.3;
 }
 
 void implicitConversionFromBoolLiterals() {
   functionTaking<int>(true);
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion bool -> 'int'
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'int'
   // CHECK-FIXES: functionTaking<int>(1);
 
   functionTaking<unsigned long>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'unsigned long'
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'bool' -> 'unsigned long'
   // CHECK-FIXES: functionTaking<unsigned long>(0u);
 
   functionTaking<signed char>(true);
-  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: implicit conversion bool -> 'signed char'
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: implicit conversion 'bool' -> 'signed char'
   // CHECK-FIXES: functionTaking<signed char>(1);
 
   functionTaking<float>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion bool -> 'float'
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'bool' -> 'float'
   // CHECK-FIXES: functionTaking<float>(0.0f);
 
   functionTaking<double>(true);
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion bool -> 'double'
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion 'bool' -> 'double'
   // CHECK-FIXES: functionTaking<double>(1.0);
 }
 
@@ -118,11 +118,11 @@ void implicitConversionFromBoolInComparisons() {
   int integer = 0;
 
   functionTaking<bool>(boolean == integer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'int'
   // CHECK-FIXES: functionTaking<bool>(static_cast<int>(boolean) == integer);
 
   functionTaking<bool>(integer != boolean);
-  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: implicit conversion bool -> 'int'
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: implicit conversion 'bool' -> 'int'
   // CHECK-FIXES: functionTaking<bool>(integer != static_cast<int>(boolean));
 }
 
@@ -171,59 +171,59 @@ void useOfTemplateFunction() {
 void implicitConversionToBoolSimpleCases() {
   int integer = 10;
   functionTaking<bool>(integer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(integer != 0);
 
   unsigned long unsignedLong = 10;
   functionTaking<bool>(unsignedLong);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(unsignedLong != 0u);
 
   float floating = 0.0f;
   functionTaking<bool>(floating);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(floating != 0.0f);
 
   double doubleFloating = 1.0f;
   functionTaking<bool>(doubleFloating);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(doubleFloating != 0.0);
 
   signed char character = 'a';
   functionTaking<bool>(character);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'signed char' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'signed char' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(character != 0);
 
   int* pointer = nullptr;
   functionTaking<bool>(pointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(pointer != nullptr);
 
   auto pointerToMember = &Struct::member;
   functionTaking<bool>(pointerToMember);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(pointerToMember != nullptr);
 }
 
 void implicitConversionToBoolInSingleExpressions() {
   int integer = 10;
   bool boolComingFromInt = integer;
-  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'int' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'int' -> 'bool'
   // CHECK-FIXES: bool boolComingFromInt = integer != 0;
 
   float floating = 10.0f;
   bool boolComingFromFloat = floating;
-  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'float' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'float' -> 'bool'
   // CHECK-FIXES: bool boolComingFromFloat = floating != 0.0f;
 
   signed char character = 'a';
   bool boolComingFromChar = character;
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: implicit conversion 'signed char' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: implicit conversion 'signed char' -> 'bool'
   // CHECK-FIXES: bool boolComingFromChar = character != 0;
 
   int* pointer = nullptr;
   bool boolComingFromPointer = pointer;
-  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int *' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int *' -> 'bool'
   // CHECK-FIXES: bool boolComingFromPointer = pointer != nullptr;
 }
 
@@ -233,131 +233,131 @@ void implicitConversionToBoolInComplexExpressions() {
   int integer = 10;
   int anotherInteger = 20;
   bool boolComingFromInteger = integer + anotherInteger;
-  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int' -> 'bool'
...
[truncated]

felix642 · 2024-01-15T18:01:32Z

LGTM !
You'll need to update your branch with master but the PR looks good.

The only thing that seems odd is the changes that you've made to the method areParensNeededForOverloadedOperator where the Operators OO_New, OO_Delete, OO_Array_New and OO_ArrayDelete now need parenthesis where they did not before.
I'm assuming that you have a used case in mind where parenthesis would be needed and this is why you've changed the behaviour.

…onversion-applies-wrong-fix-in-ternary-operator

PiotrZSL · 2024-01-15T18:52:46Z

The thing with OO_New and other was that when i merged code and extracted areParensNeededForStatement simple because I know that it will be needed for other checks I based it more on https://en.cppreference.com/w/cpp/language/operator_precedence. But there many operators are in same group, and information that is +- missing is an new parent operator. In future maybe areParensNeededForStatement will be extended to cover both old and new parent operator and provide more accurate hints. To avoid potential issues I put those OO_New and other on list that they need (), simply because that don't cause any problems, and to be honest there are not many use-cases and most of them are invalid anyway from a user perspective (like implicit casting of newly allocated memory to bool).

…version fixes (llvm#74891) Check now more properly add missing parentheses to code like this: 'bool bar = true ? 1 : 0 != 0;'. Closes llvm#71867

[clang-tidy] Fix missing parentheses in readability-implicit-bool-con…

8c3b797

…version fixes Check now more properly add missing parentheses to code like this: 'bool bar = true ? 1 : 0 != 0;'.

PiotrZSL added the clang-tidy label Dec 8, 2023

PiotrZSL self-assigned this Dec 8, 2023

felix642 approved these changes Jan 15, 2024

View reviewed changes

Merge branch 'main' into 71867-clang-tidy-readability-implicit-bool-c…

e6fff3b

…onversion-applies-wrong-fix-in-ternary-operator

felix642 approved these changes Jan 15, 2024

View reviewed changes

PiotrZSL merged commit fd0e06d into llvm:main Jan 16, 2024
5 checks passed

PiotrZSL deleted the 71867-clang-tidy-readability-implicit-bool-conversion-applies-wrong-fix-in-ternary-operator branch January 16, 2024 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes #74891

[clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes #74891

PiotrZSL commented Dec 8, 2023

llvmbot commented Dec 8, 2023

felix642 commented Jan 15, 2024

PiotrZSL commented Jan 15, 2024

[clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes #74891

[clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes #74891

Conversation

PiotrZSL commented Dec 8, 2023

llvmbot commented Dec 8, 2023

felix642 commented Jan 15, 2024

PiotrZSL commented Jan 15, 2024