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

[via cfe-commits]

Author: Congcong Cai
Date: 2025-01-11T18:48:39+08:00
New Revision: 32bcd41adcc664f6d690efc9b7cd209ac9c65f68

URL: https://github.com/llvm/llvm-project/commit/32bcd41adcc664f6d690efc9b7cd209ac9c65f68
DIFF: https://github.com/llvm/llvm-project/commit/32bcd41adcc664f6d690efc9b7cd209ac9c65f68.diff

LOG: [clang-tidy] use correct template type in ``std::min`` and ``std::max`` when operand is integer literal for readability-use-std-min-max (#122296)

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);
```

Fixed: #121676

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp

Removed: 
    


################################################################################
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 9cdad8fb6b58f2..d75a276729c581 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -369,6 +369,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/redundant-smartptr-get>` check to
   remove `->`, when redundant `get()` is removed.
 
+- 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