Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max #77816
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… with std::min/std::max Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
|
@llvm/pr-subscribers-clang-tidy Author: Bhuminjay Soni (11happy) ChangesOverview: Testing:
Dependencies:
CC:
Full diff: https://github.com/llvm/llvm-project/pull/77816.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 408c822b861c5f..4bc373bb69bb84 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangTidyReadabilityModule
AvoidReturnWithVoidValueCheck.cpp
AvoidUnconditionalPreprocessorIfCheck.cpp
BracesAroundStatementsCheck.cpp
+ ConditionaltostdminmaxCheck.cpp
ConstReturnTypeCheck.cpp
ContainerContainsCheck.cpp
ContainerDataPointerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
new file mode 100644
index 00000000000000..86420765d6f6c6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
@@ -0,0 +1,88 @@
+//===--- ConditionaltostdminmaxCheck.cpp - clang-tidy ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConditionaltostdminmaxCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void ConditionaltostdminmaxCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ ifStmt(has(binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">")),
+ hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))),
+ hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))))),
+ hasThen(stmt(binaryOperator(
+ hasOperatorName("="),
+ hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))),
+ hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2")))))))
+ .bind("ifStmt"),
+ this);
+}
+
+void ConditionaltostdminmaxCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const DeclRefExpr *lhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar1");
+ const DeclRefExpr *rhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar1");
+ const DeclRefExpr *lhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar2");
+ const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2");
+ const IfStmt *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt");
+
+ if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt)
+ return;
+
+ const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond());
+ if (!binaryOp)
+ return;
+
+ SourceLocation ifLocation = ifStmt->getIfLoc();
+ SourceLocation thenLocation = ifStmt->getEndLoc();
+
+ if (binaryOp->getOpcode() == BO_LT) {
+ if (lhsVar1->getDecl() == lhsVar2->getDecl() &&
+ rhsVar1->getDecl() == rhsVar2->getDecl()) {
+ diag(ifStmt->getIfLoc(), "use std::max instead of <")
+ << FixItHint::CreateReplacement(
+ SourceRange(ifLocation, thenLocation),
+ lhsVar2->getNameInfo().getAsString() + " = std::max(" +
+ lhsVar1->getNameInfo().getAsString() + ", " +
+ rhsVar1->getNameInfo().getAsString() + ")");
+ } else if (lhsVar1->getDecl() == rhsVar2->getDecl() &&
+ rhsVar1->getDecl() == lhsVar2->getDecl()) {
+ diag(ifStmt->getIfLoc(), "use std::min instead of <")
+ << FixItHint::CreateReplacement(
+ SourceRange(ifLocation, thenLocation),
+ lhsVar2->getNameInfo().getAsString() + " = std::min(" +
+ lhsVar1->getNameInfo().getAsString() + ", " +
+ rhsVar1->getNameInfo().getAsString() + ")");
+ }
+ } else if (binaryOp->getOpcode() == BO_GT) {
+ if (lhsVar1->getDecl() == lhsVar2->getDecl() &&
+ rhsVar1->getDecl() == rhsVar2->getDecl()) {
+ diag(ifStmt->getIfLoc(), "use std::min instead of >")
+ << FixItHint::CreateReplacement(
+ SourceRange(ifLocation, thenLocation),
+ lhsVar2->getNameInfo().getAsString() + " = std::min(" +
+ lhsVar1->getNameInfo().getAsString() + ", " +
+ rhsVar1->getNameInfo().getAsString() + ")");
+ } else if (lhsVar1->getDecl() == rhsVar2->getDecl() &&
+ rhsVar1->getDecl() == lhsVar2->getDecl()) {
+ diag(ifStmt->getIfLoc(), "use std::max instead of >")
+ << FixItHint::CreateReplacement(
+ SourceRange(ifLocation, thenLocation),
+ lhsVar2->getNameInfo().getAsString() + " = std::max(" +
+ lhsVar1->getNameInfo().getAsString() + ", " +
+ rhsVar1->getNameInfo().getAsString() + ")");
+ }
+ }
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h
new file mode 100644
index 00000000000000..02ebed83fed6c8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h
@@ -0,0 +1,32 @@
+//===--- ConditionaltostdminmaxCheck.h - clang-tidy -------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// FIXME: replaces certain conditional statements with equivalent std::min or
+/// std::max expressions, improving readability and promoting the use of
+/// standard library functions."
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/ConditionalToStdMinMax.html
+class ConditionaltostdminmaxCheck : public ClangTidyCheck {
+public:
+ ConditionaltostdminmaxCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 0b0aad7c0dcb36..63f8f03be6bb91 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidReturnWithVoidValueCheck.h"
#include "AvoidUnconditionalPreprocessorIfCheck.h"
#include "BracesAroundStatementsCheck.h"
+#include "ConditionaltostdminmaxCheck.h"
#include "ConstReturnTypeCheck.h"
#include "ContainerContainsCheck.h"
#include "ContainerDataPointerCheck.h"
@@ -62,6 +63,8 @@ namespace readability {
class ReadabilityModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<ConditionaltostdminmaxCheck>(
+ "readability-ConditionalToStdMinMax");
CheckFactories.registerCheck<AvoidConstParamsInDecls>(
"readability-avoid-const-params-in-decls");
CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b4d87e0ed2a67a..c3fb990ad6a738 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,12 @@ New checks
Recommends the smallest possible underlying type for an ``enum`` or ``enum``
class based on the range of its enumerators.
+- New :doc:`readability-ConditionalToStdMinMax
+ <clang-tidy/checks/readability/ConditionalToStdMinMax>` check.
+
+ Replaces certain conditional statements with equivalent std::min or std::max expressions,
+ improving readability and promoting the use of standard library functions.
+
- New :doc:`readability-reference-to-constructed-temporary
<clang-tidy/checks/readability/reference-to-constructed-temporary>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 2f86121ad87299..c2eac55425c69e 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -336,6 +336,7 @@ Clang-Tidy Checks
:doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes"
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
+ :doc:`readability-ConditionalToStdMinMax <readability/ConditionalToStdMinMax>`, "Yes"
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`,
:doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
new file mode 100644
index 00000000000000..e95858999b2772
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-ConditionalToStdMinMax
+
+readability-ConditionalToStdMinMax
+==================================
+
+Replaces certain conditional statements with equivalent std::min or std::max expressions,
+improving readability and promoting the use of standard library functions.
+
+Examples:
+
+Before:
+
+.. code-block:: c++
+
+ void foo(){
+ int a,b;
+ if(a < b)
+ a = b;
+ }
+
+
+After:
+
+.. code-block:: c++
+
+ int main(){
+ a = std::max(a, b);
+
+ }
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
new file mode 100644
index 00000000000000..d29e9aa9fec708
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s readability-ConditionalToStdMinMax %t
+
+void foo() {
+ int value1,value2;
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of < [readability-ConditionalToStdMinMax]
+ if (value1 < value2)
+ value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2);
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of < [readability-ConditionalToStdMinMax]
+ if (value1 < value2)
+ value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2);
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of > [readability-ConditionalToStdMinMax]
+ if (value2 > value1)
+ value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1);
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of > [readability-ConditionalToStdMinMax]
+ if (value2 > value1)
+ value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1);
+
+ // No suggestion needed here
+ if (value1 == value2)
+ value1 = value2;
+
+
+}
\ No newline at end of file
|
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
Outdated
Show resolved
Hide resolved
PiotrZSL
reviewed
Jan 11, 2024
There was a problem hiding this comment.
Some tweaks needed. Note that there is also issue for:
std::max(a, std::max(b, c)) -> std::max({a, b, c})
and this check would be nice thing for that also, but not under this PR.
Overall you got something that works, but is very strict, now I would like to see it to become more .. generic and could land.
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp
Outdated
Show resolved
Hide resolved
|
Thank you for the suggestions and your guidance I will update it as soon as possible. |
felix642
reviewed
Jan 11, 2024
clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp
Outdated
Show resolved
Hide resolved
|
LLVM uses |
|
Thank you for all the suggestions , I think I have tried to incorporate all of them . |
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Show resolved
Hide resolved
Signed-off-by: 11happy <soni5happy@gmail.com>
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: 11happy <soni5happy@gmail.com>
|
@PiotrZSL can you please review the changes. |
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: 11happy <soni5happy@gmail.com>
|
I have added the initialisation of variables and also added tests involving constexpr,functions etc. |
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
PiotrZSL
requested changes
Jan 28, 2024
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Outdated
Show resolved
Hide resolved
PiotrZSL
reviewed
Jan 28, 2024
clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
Outdated
Show resolved
Hide resolved
PiotrZSL
reviewed
Jan 29, 2024
There was a problem hiding this comment.
Other thing is that reviewer should mark issues as resolved after checking with code, not author. Otherwise is hard to figure out what's done and what's not. Adding some comment like "Done" is sufficient usually unless issues are simple.
Next thing regarding your 2 comments that somehow cannot find in GitHub and exist only in my mail box:
- vector size
std::vector<std::vector<Foo>> a;
unsigned int b = a[0].size();
if(a[0].size() > b)
b = a[0].size();
first, you do not have a test for that, like:
using my_size = unsigned long long;
template<typename T>
struct MyVector
{
using size_type = my_size;
size_type size() const;
};
void testVectorSizeType() {
MyVector<int> v;
unsigned int value;
if (v.size() > value)
value = v.size();
if (value < v.size())
value = v.size();
}
Second, in this case check in current state work fine and will put unsigned long long as a type, the only problem is that unsigned long long is ugly.
ElaboratedType 0x55cfafb2f390 'size_type' sugar
`-TypedefType 0x55cfafb2f360 'MyVector::size_type' sugar
|-TypeAlias 0x55cfafb2f300 'size_type'
`-ElaboratedType 0x55cfafb2f2c0 'my_size' sugar
`-TypedefType 0x55cfafb2f290 'my_size' sugar
|-TypeAlias 0x55cfafb2d900 'my_size'
`-BuiltinType 0x55cfafabb710 'unsigned long long'
This is multi level typedef, proper solution would be to drop elaborated type until we get into alias that isn't template instance and isn't in template instance and isn't in template..
You can look into readability-redundant-casting how to drop some levels of elaborated types. And use that instead of getCanonicalType.
value1 + value2 < valuex
You should check only that binary operator that check trigger:
IfStmt 0x56422decb568 <line:256:3, line:257:23>
|-BinaryOperator 0x56422decb430 <line:256:6, col:24> 'bool' '<'
| |-ImplicitCastExpr 0x56422decb418 <col:6, col:15> 'unsigned int' <IntegralCast>
| | `-BinaryOperator 0x56422decb3c0 <col:6, col:15> 'int' '+'
| | |-ImplicitCastExpr 0x56422decb378 <col:6> 'int' <IntegralCast>
| | | `-ImplicitCastExpr 0x56422decb360 <col:6> 'unsigned char' <LValueToRValue>
| | | `-DeclRefExpr 0x56422decb320 <col:6> 'unsigned char' lvalue Var 0x56422decb0c8 'value1' 'unsigned char'
| | `-ImplicitCastExpr 0x56422decb3a8 <col:15> 'int' <IntegralCast>
| | `-ImplicitCastExpr 0x56422decb390 <col:15> 'short' <LValueToRValue>
| | `-DeclRefExpr 0x56422decb340 <col:15> 'short' lvalue Var 0x56422decb198 'value2' 'short'
| `-ImplicitCastExpr 0x56422decb400 <col:24> 'unsigned int' <LValueToRValue>
| `-DeclRefExpr 0x56422decb3e0 <col:24> 'unsigned int' lvalue Var 0x56422decb268 'valuex' 'unsigned int'
`-BinaryOperator 0x56422decb548 <line:257:5, col:23> 'unsigned int' lvalue '='
|-DeclRefExpr 0x56422decb450 <col:5> 'unsigned int' lvalue Var 0x56422decb268 'valuex' 'unsigned int'
`-ImplicitCastExpr 0x56422decb530 <col:14, col:23> 'unsigned int' <IntegralCast>
`-BinaryOperator 0x56422decb510 <col:14, col:23> 'int' '+'
|-ImplicitCastExpr 0x56422decb4c8 <col:14> 'int' <IntegralCast>
| `-ImplicitCastExpr 0x56422decb4b0 <col:14> 'unsigned char' <LValueToRValue>
| `-DeclRefExpr 0x56422decb470 <col:14> 'unsigned char' lvalue Var 0x56422decb0c8 'value1' 'unsigned char'
`-ImplicitCastExpr 0x56422decb4f8 <col:23> 'int' <IntegralCast>
`-ImplicitCastExpr 0x56422decb4e0 <col:23> 'short' <LValueToRValue>
`-DeclRefExpr 0x56422decb490 <col:23> 'short' lvalue Var 0x56422decb198 'value2' 'short'
in this case we got: unsigned char + short < unsigned int, first part will be cased to unsigned int even if ``unsigned char + shortgives int, and that's fine. Current version of check handles that well,GlobalImplicitCastType = BO->getLHS()->getType();` does a trick.
@PiotrZSL I have attempted to implement this suggestion, but I am encountering difficulties and confusion due to the various types involved. Could you kindly provide some guidance or assistance on how to effectively simplify the type hierarchy in this context? I have explored methods like getDesugaredType() but would appreciate your guidance specifically |
Signed-off-by: 11happy <soni5happy@gmail.com>
|
@PiotrZSL , I have implemented the solution you suggested, took me some time though , but I can say it got me better understanding of how things work. Thank you |
|
Humble Ping! (as per the rules ping after week of inactivity) @PiotrZSL , @5chmidti , @felix642 , @EugeneZelenko , I think its nearly done. |
PiotrZSL
approved these changes
Feb 4, 2024
felix642
approved these changes
Feb 4, 2024
5chmidti
approved these changes
Feb 5, 2024
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
EugeneZelenko
approved these changes
Feb 5, 2024
|
Unless you want this to be merged as "76656712+11happy@users.noreply.github.com", change your email privacy settings. |
I have changed the settings. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview:
This pull request fixes #64914 where author suggests adding a readability check to propose the replacement of conditional statements with std::min/std::max for improved code readability. Additionally, reference is made to PyLint's similar checks: consider-using-min-builtin and consider-using-max-builtin
Testing:
Dependencies:
CC: