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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
