Skip to content

Commit

Permalink
[clang-tidy] use correct template type in std::min and ``std::max…
Browse files Browse the repository at this point in the history
…`` 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
  • Loading branch information
HerrCai0907 authored Jan 11, 2025
1 parent 642e493 commit 32bcd41
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
35 changes: 23 additions & 12 deletions clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 32bcd41

Please sign in to comment.