PiotrZSL added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:57 + + const SourceManager &SM = Result.Context->getSourceManager(); + SourceLocation RAngleLoc = ---------------- Result got source manager.... https://clang.llvm.org/doxygen/structclang_1_1ast__matchers_1_1MatchFinder_1_1MatchResult.html ================ Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:61-62 + + FixItHint TypenameHint = + FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(), "typename "); + FixItHint TypeHint = ---------------- do we still need typename in C++20 in this context ? https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0634r3.html ================ Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:67 + diag(EnableIf->getBeginLoc(), "incorrect std::enable_if usage detected; use " + "'typename std::enable_if<...>::type'") + << TypenameHint << TypeHint; ---------------- since C++14 we should recommend using enable_if_t ================ Comment at: clang-tools-extra/docs/ReleaseNotes.rst:129 + + Detects incorrect usages of std::enable_if that don't name the nested 'type' + type. ---------------- ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:21 + + template <typename T, typename = typename std::enable_if_t<T::some_trait>::type> + void valid_usage() { ... } ---------------- shoudnlt be enable_if ? ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:34 + template <typename T, typename = std::enable_if_t<T::some_trait>> + void invalid_usage() { ... } + ---------------- this is valid usage, uses enable_if_t ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:37 + // The tool suggests the following replacement for 'invalid_usage': + template <typename T, typename = typename std::enable_if_t<T::some_trait>::type> + void invalid_usage() { ... } ---------------- this may not compile, anyway, what if "type" got "type" ?:P ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:44 +`modernize-type-traits <../modernize/type-traits.html>`_ for another tool +that will replace ``typename std::enable_if<...>::type`` with +``std::enable_if_t<...>``, and see ---------------- should be suffucient ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:47 +`modernize-use-constraints <../modernize/use-constraints.html>`_ for another +tool that replaces ``std::enable_if`` with C++20 constraints. Use these +newer mechanisms where possible. ---------------- maybe "Consider" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits