[clang-tools-extra] [clang-tidy] Fix invalid code generation for member typedefs in readability-use-std-min-max (PR #208762)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


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

@llvm/pr-subscribers-clang-tidy

Author: 歌德 (xxxxbc)

<details>
<summary>Changes</summary>

## Summary

- When the two compared operands have different types, `readability-use-std-min-max` emits an explicit template argument for `std::min`/`std::max`. For class member typedefs (e.g. `std::string::size_type`), the bare typedef name is not in scope at the fix location, producing code that does not compile.
- Fix `getNonTemplateAlias()` to keep desugaring when the typedef is declared inside a record (class/struct), so the underlying type is used instead.
- Chose the underlying-type approach over `decltype(expr)` to keep the fix minimal and avoid churning existing test expectations. Maintainer feedback on this choice is welcome.

Fixes #<!-- -->208693.

## Test plan

- Added regression test in `gh208693` namespace with a reduced repro from the issue
- Verified existing tests (`my_size`, `U16`, `int`, `unsigned int`) are unaffected
- `llvm-lit .../readability/use-std-min-max.cpp` passes
- Manual verification: repro rewrites to `std::max<unsigned long>(...)` and compiles clean


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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp (+7-3) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp (+17) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
index ed1af05d232ce..ee784e764613e 100644
--- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp
@@ -63,9 +63,13 @@ static QualType getNonTemplateAlias(QualType QT) {
   while (true) {
     // cast to a TypedefType
     if (const auto *TT = dyn_cast<TypedefType>(QT)) {
-      // check if the typedef is a template and if it is dependent
-      if (!TT->getDecl()->getDescribedTemplate() &&
-          !TT->getDecl()->getDeclContext()->isDependentContext())
+      const TypedefNameDecl *TD = TT->getDecl();
+      // Check if the typedef is a template and if it is dependent. A class
+      // member typedef (e.g. std::string::size_type) is not usable by its
+      // bare name at the location of the fix, so keep desugaring in that case.
+      if (!TD->getDescribedTemplate() &&
+          !TD->getDeclContext()->isDependentContext() &&
+          !TD->getDeclContext()->isRecord())
         return QT;
       QT = TT->desugar();
     } else {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 082cc5cb78bc0..1e71015a065b1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -915,6 +915,12 @@ Changes in existing checks
   note to suggest materializing the temporary range when iterating over temporary
   range expressions or initializer lists, as reusing them directly could be unsafe.
 
+- Improved :doc:`readability-use-std-min-max
+  <clang-tidy/checks/readability/use-std-min-max>` check by fixing invalid code
+  generation when the explicit template argument resolved to a class member
+  typedef (for example ``std::string::size_type``); the underlying type is now
+  used instead.
+
 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 35570189e1122..1f7c6bb56a889 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
@@ -253,6 +253,23 @@ void testVectorSizeType() {
     value = v.size();
 }
 
+namespace gh208693 {
+struct B {
+protected:
+  typedef unsigned long bsize;
+};
+struct S : B {
+  typedef B::bsize size_type;
+  size_type size() const;
+};
+void f(const S &s, unsigned &n) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max]
+  // CHECK-FIXES: n = std::max<unsigned long>(s.size(), n);
+  if (s.size() > n)
+    n = s.size();
+}
+} // namespace gh208693
+
 namespace gh121676 {
 
 void useLeft() {

``````````

</details>


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