[PATCH] D122086: [mlir:PDL] Expand how native constraint/rewrite functions can be defined

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar accepted this revision.
jpienaar added a comment.

Some template magic indeed, but the results are much more user friendly, thanks!



================
Comment at: mlir/include/mlir/IR/PatternMatch.h:821
+///
+///  template <typename T>
+/// void foo() {
----------------
Micro nit: extra space :)

Doesn't some of these also go away with C++17? (I could be confused)


================
Comment at: mlir/include/mlir/IR/PatternMatch.h:916
+              PDLValue pdlValue, size_t argIdx) {
+    if (!pdlValue) {
+      return errorFn("expected a non-null value for argument " + Twine(argIdx) +
----------------
Flip order?

if (pdlValue) return success(); 
return ... ?


================
Comment at: mlir/include/mlir/IR/PatternMatch.h:979
+    static_assert(always_false<T>,
+                  "Prefer using StringRef for string-like arguments");
+  }
----------------
Is this only a preference or a requirement given what is stored?


================
Comment at: mlir/include/mlir/IR/PatternMatch.h:1128
+    (void)std::initializer_list<int>{
+        0, (processResults(rewriter, results, std::move(args)), 0)...};
+  };
----------------
I don't quite follow the 0's here (I may just not be following the logic at all, could you add short explanation?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122086