[clang-tidy][NFC] Enable readability-avoid-return-with-void-value check in the codebase

[localspook]

No description provided.

[llvmbot]

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

@llvm/pr-subscribers-clang-tidy

Author: Victor Chernyakin (localspook)

Changes

---

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

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/.clang-tidy (-1)
• (modified) clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp (+42-32)
• (modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp (+3-2)

diff --git a/clang-tools-extra/clang-tidy/.clang-tidy b/clang-tools-extra/clang-tidy/.clang-tidy
index 2443c979621da..0a2ea24cb5f73 100644
--- a/clang-tools-extra/clang-tidy/.clang-tidy
+++ b/clang-tools-extra/clang-tidy/.clang-tidy
@@ -22,7 +22,6 @@ Checks: >
   -performance-unnecessary-value-param,
   readability-*,
   -readability-avoid-nested-conditional-operator,
-  -readability-avoid-return-with-void-value,
   -readability-braces-around-statements,
   -readability-container-contains,
   -readability-convert-member-functions-to-static,
diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
index 8b2ca6968ea75..a9d0aedd9aceb 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
@@ -381,10 +381,11 @@ void NarrowingConversionsCheck::diagNarrowTypeOrConstant(
     const Expr &Rhs) {
   APValue Constant = getConstantExprValue(Context, Rhs);
   if (Constant.isInt())
-    return diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, Constant.getInt());
-  if (Constant.isFloat())
-    return diagNarrowConstant(SourceLoc, Lhs, Rhs);
-  return diagNarrowType(SourceLoc, Lhs, Rhs);
+    diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, Constant.getInt());
+  else if (Constant.isFloat())
+    diagNarrowConstant(SourceLoc, Lhs, Rhs);
+  else
+    diagNarrowType(SourceLoc, Lhs, Rhs);
 }
 
 void NarrowingConversionsCheck::handleIntegralCast(const ASTContext &Context,
@@ -460,10 +461,10 @@ void NarrowingConversionsCheck::handleFloatingToIntegral(
   llvm::APFloat FloatConstant(0.0);
   if (getFloatingConstantExprValue(Context, Rhs, FloatConstant)) {
     if (!isFloatExactlyRepresentable(Context, FloatConstant, Lhs.getType()))
-      return diagNarrowConstant(SourceLoc, Lhs, Rhs);
+      diagNarrowConstant(SourceLoc, Lhs, Rhs);
 
-    if (PedanticMode)
-      return diagConstantCast(SourceLoc, Lhs, Rhs);
+    else if (PedanticMode)
+      diagConstantCast(SourceLoc, Lhs, Rhs);
 
     return;
   }
@@ -478,7 +479,7 @@ void NarrowingConversionsCheck::handleFloatingToIntegral(
 void NarrowingConversionsCheck::handleFloatingToBoolean(
     const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
     const Expr &Rhs) {
-  return diagNarrowTypeOrConstant(Context, SourceLoc, Lhs, Rhs);
+  diagNarrowTypeOrConstant(Context, SourceLoc, Lhs, Rhs);
 }
 
 void NarrowingConversionsCheck::handleBooleanToSignedIntegral(
@@ -532,19 +533,20 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context,
   if (LhsType == RhsType)
     return;
   if (RhsType->getKind() == BuiltinType::Bool && LhsType->isSignedInteger())
-    return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isInteger() && LhsType->getKind() == BuiltinType::Bool)
-    return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isInteger() && LhsType->isFloatingPoint())
-    return handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isInteger() && LhsType->isInteger())
-    return handleIntegralCast(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isFloatingPoint() && LhsType->getKind() == BuiltinType::Bool)
-    return handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isFloatingPoint() && LhsType->isInteger())
-    return handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs);
-  if (RhsType->isFloatingPoint() && LhsType->isFloatingPoint())
-    return handleFloatingCast(Context, SourceLoc, Lhs, Rhs);
+    handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isInteger() && LhsType->getKind() == BuiltinType::Bool)
+    handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isInteger() && LhsType->isFloatingPoint())
+    handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isInteger() && LhsType->isInteger())
+    handleIntegralCast(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isFloatingPoint() &&
+           LhsType->getKind() == BuiltinType::Bool)
+    handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isFloatingPoint() && LhsType->isInteger())
+    handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs);
+  else if (RhsType->isFloatingPoint() && LhsType->isFloatingPoint())
+    handleFloatingCast(Context, SourceLoc, Lhs, Rhs);
 }
 
 bool NarrowingConversionsCheck::handleConditionalOperator(
@@ -577,19 +579,26 @@ void NarrowingConversionsCheck::handleImplicitCast(
   SourceLocation SourceLoc = Lhs.getExprLoc();
   switch (Cast.getCastKind()) {
   case CK_BooleanToSignedIntegral:
-    return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
+    handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_IntegralToBoolean:
-    return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);
+    handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_IntegralToFloating:
-    return handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs);
+    handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_IntegralCast:
-    return handleIntegralCast(Context, SourceLoc, Lhs, Rhs);
+    handleIntegralCast(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_FloatingToBoolean:
-    return handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs);
+    handleFloatingToBoolean(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_FloatingToIntegral:
-    return handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs);
+    handleFloatingToIntegral(Context, SourceLoc, Lhs, Rhs);
+    break;
   case CK_FloatingCast:
-    return handleFloatingCast(Context, SourceLoc, Lhs, Rhs);
+    handleFloatingCast(Context, SourceLoc, Lhs, Rhs);
+    break;
   default:
     break;
   }
@@ -610,9 +619,10 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context,
 
 void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("binary_op"))
-    return handleBinaryOperator(*Result.Context, *Op);
-  if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast"))
-    return handleImplicitCast(*Result.Context, *Cast);
-  llvm_unreachable("must be binary operator or cast expression");
+    handleBinaryOperator(*Result.Context, *Op);
+  else if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast"))
+    handleImplicitCast(*Result.Context, *Cast);
+  else
+    llvm_unreachable("must be binary operator or cast expression");
 }
 } // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 20c73299915a9..5f19706e16866 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -361,14 +361,15 @@ void ImplicitBoolConversionCheck::check(
   if (const auto *CastToBool =
           Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastToBool")) {
     const auto *Parent = Result.Nodes.getNodeAs<Stmt>("parentStmt");
-    return handleCastToBool(CastToBool, Parent, *Result.Context);
+    handleCastToBool(CastToBool, Parent, *Result.Context);
+    return;
   }
 
   if (const auto *CastFromBool =
           Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastFromBool")) {
     const auto *NextImplicitCast =
         Result.Nodes.getNodeAs<ImplicitCastExpr>("furtherImplicitCast");
-    return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context);
+    handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context);
   }
 }
 

[vbvictor]

Thank you for working on this!
LGTM with couple nits