[PATCH] D81045: [LLVM] Change isa<> to a variadic function template

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 10 11:41:21 PDT 2020


rriddle accepted this revision.
rriddle marked an inline comment as done.
rriddle added a comment.
This revision is now accepted and ready to land.

I have run into the desire to have this on many occasions, so LGTM for me.



================
Comment at: clang/include/clang/AST/DeclBase.h:521
     AttrVec &Vec = getAttrs();
-    Vec.erase(std::remove_if(Vec.begin(), Vec.end(), isa<T, Attr*>), Vec.end());
+    Vec.erase(std::remove_if(Vec.begin(), Vec.end(),
+                             [](Attr *A) { return isa<T>(A); }),
----------------
nit: We could also update this to use `llvm::erase_if`, but fine to leave as is.


================
Comment at: llvm/include/llvm/Support/Casting.h:147
+template <typename First, typename... Rest, typename Y>
+LLVM_NODISCARD inline typename std::enable_if<sizeof...(Rest) != 0, bool>::type
+isa(const Y &Val) {
----------------
nit: I would remove the enable_if and just add an additional type to the template.

```
template<typename First, typename Second, ...>
... isa(const Y &val) {
  return isa<First>(Val) || isa<Second, Rest...>(Val);
}
```


================
Comment at: llvm/include/llvm/Support/Casting.h:136
+// isa<X> - Return true if the parameter to the template is an instance of one
+// of the template type argument.  Used like this:
 //
----------------
nit: arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81045





More information about the llvm-commits mailing list