[PATCH] D141892: [clang-tidy] Implement modernize-use-constraints

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

Overall this check is complicated, simply because it's doing complicated stuff.

1. Add description to all functions (what is their purpose, what is expected outcome, maybe put some "example")
2. Extend documentation, add "supported" constructions, (pre C++20, C++20).
3. What about boost enable_if ? Support it, or restrict check to std only.
4. What about enable_if used as an function argument ? Support it, or add some info to documentation that such construction is not supported.

I don't think that this check will ever be "perfect", so LGTM.
We got entire release to "stabilize it".



================
Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:54
+  if (const auto Dep = TheType.getAs<DependentNameTypeLoc>()) {
+    if (Dep.getTypePtr()->getIdentifier()->getName() != "type" ||
+        Dep.getTypePtr()->getKeyword() != ETK_Typename) {
----------------
what if we do not have Identifier here ?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:205-208
+  } else {
+    return SourceRange(EnableIf.getLAngleLoc().getLocWithOffset(1),
+                       getRAngleFileLoc(SM, EnableIf));
+  }
----------------



================
Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:352
+  FixIts.push_back(FixItHint::CreateInsertion(
+      *ConstraintInsertionLoc, "requires " + *ConditionText + " "));
+  return FixIts;
----------------
You not checking anywhere if this optional is initialized.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst:10
+template based on type traits or other requirements. ``enable_if`` changes the
+meta-arity of the template, and has other `adverse side effects <https://open-std.org/JTC1/SC22/WG21/docs/papers/2016/p0225r0.html>`_
+in the code. C++20 introduces concepts and constraints as a cleaner language
----------------
move this link to new line, 80 characters limit


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize/use-constraints.rst:20-24
+  template <typename T>
+  std::enable_if_t<T::some_trait, int> only_if_t_has_the_trait() { ... }
+
+  template <typename T, std::enable_if_t<T::some_trait, int> = 0>
+  void another_version() { ... }
----------------
Put into documentation also example how this code should look in C++20.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141892/new/

https://reviews.llvm.org/D141892