[llvm-branch-commits] [clang-tools-extra] [clang-tidy] use correct template type in ``std::min`` and ``std::max`` when operand is integer literal for readability-use-std-min-max (PR #122296)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

<details>
<summary>Changes</summary>

When comparing with integer literal, integer promote will happen to promote type which has less bit width than int to int or unsigned int.
It will let auto-fix provide correct but out of expected fix.

e.g.
```c++
short a;
if ( a > 10 )
  a = 10;
```
will be
```c++
short a;
if ( (int)a > 10 )
  a = (short)10;
```

which will be fixed as
```c++
short a;
a = std::max<int>(a, 10);
```

but actually it can be
```c++
short a;
a = std::max<short>(a, 10);
```

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp (+23-12) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp (+21) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index 179173502a8d01..6f6b8a853a91e0 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -79,6 +79,27 @@ static QualType getNonTemplateAlias(QualType QT) {
   return QT;
 }
 
+static QualType getReplacementCastType(const Expr *CondLhs, const Expr *CondRhs,
+                                       QualType ComparedType) {
+  QualType LhsType = CondLhs->getType();
+  QualType RhsType = CondRhs->getType();
+  QualType LhsCanonicalType =
+      LhsType.getCanonicalType().getNonReferenceType().getUnqualifiedType();
+  QualType RhsCanonicalType =
+      RhsType.getCanonicalType().getNonReferenceType().getUnqualifiedType();
+  QualType GlobalImplicitCastType;
+  if (LhsCanonicalType != RhsCanonicalType) {
+    if (llvm::isa<IntegerLiteral>(CondRhs)) {
+      GlobalImplicitCastType = getNonTemplateAlias(LhsType);
+    } else if (llvm::isa<IntegerLiteral>(CondLhs)) {
+      GlobalImplicitCastType = getNonTemplateAlias(RhsType);
+    } else {
+      GlobalImplicitCastType = getNonTemplateAlias(ComparedType);
+    }
+  }
+  return GlobalImplicitCastType;
+}
+
 static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
                                      const Expr *AssignLhs,
                                      const SourceManager &Source,
@@ -92,18 +113,8 @@ static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
   const llvm::StringRef AssignLhsStr = Lexer::getSourceText(
       Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
 
-  QualType GlobalImplicitCastType;
-  QualType LhsType = CondLhs->getType()
-                         .getCanonicalType()
-                         .getNonReferenceType()
-                         .getUnqualifiedType();
-  QualType RhsType = CondRhs->getType()
-                         .getCanonicalType()
-                         .getNonReferenceType()
-                         .getUnqualifiedType();
-  if (LhsType != RhsType) {
-    GlobalImplicitCastType = getNonTemplateAlias(BO->getLHS()->getType());
-  }
+  QualType GlobalImplicitCastType =
+      getReplacementCastType(CondLhs, CondRhs, BO->getLHS()->getType());
 
   return (AssignLhsStr + " = " + FunctionName +
           (!GlobalImplicitCastType.isNull()
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 94e15639c4a92e..fd523da8dc5a1b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -364,6 +364,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/identifier-naming>` check to
   validate ``namespace`` aliases.
 
+- Improved :doc:`readability-use-std-min-max
+  <clang-tidy/checks/readability/use-std-min-max>` check to use correct template
+  type in ``std::min`` and ``std::max`` when operand is integer literal.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
index 9c0e2eabda348d..35ade8a7c6d37e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp
@@ -252,3 +252,24 @@ void testVectorSizeType() {
   if (value < v.size())
     value = v.size();
 }
+
+namespace gh121676 {
+
+void useLeft() {
+  using U16 = unsigned short;
+  U16 I = 0;
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max]
+  // CHECK-FIXES: I = std::max<U16>(I, 16U);
+  if (I < 16U)
+    I = 16U;
+}
+void useRight() {
+  using U16 = unsigned short;
+  U16 I = 0;
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max]
+  // CHECK-FIXES: I = std::min<U16>(16U, I);
+  if (16U < I)
+    I = 16U;
+}
+
+} // namespace gh121676

``````````

</details>


https://github.com/llvm/llvm-project/pull/122296