[PATCH] D22403: FileCheck Enhancement - pattern templates.

[Vedant Kumar via llvm-commits]

vsk added a comment.

I've added a few comments inline which I hope will be useful. However, after looking this patch over a few times, I've found that it's too large for me to review constructively.

The main issue is that the grammar you've implemented doesn't correspond to the one we discussed. E.g your PATTERN_ELEMENT includes PATTERN_USE, and your ARG is not handled correctly. I don't want to say "no" to new features, but I would much rather have them land incrementally, with more testing for edge-cases and more documentation. Can you scale back this patch to just cover these new bits of the grammar?

  ACTION <- CHECK-DEFINE-PATTERN ':' IDENT ':' PATTERN_ELEMENT* '\n' ;
  PATTERN_ELEMENT <- IDENT | REGEX;
  MATCH <- PATTERN_USE ;
  PATTERN_USE <- '[[' '@' IDENT ']]' ;
  VAR <- '[[' IDENT '@' IDENT ']]' ;


================
Comment at: docs/CommandGuide/FileCheck.rst:488
@@ +487,3 @@
+Some regular expressions are used very often. In this case directive for describing template 
+of regular expression may be used.
+
----------------
Please use the same formatting rules as the rest of this document. Can you trim the twiddles ("~~~") to match the section name's length and line-wrap text that isn't in a code block?

================
Comment at: docs/CommandGuide/FileCheck.rst:495
@@ +494,3 @@
+    // CHECK-DEFINE-PATTERN: [name of describing template]: [pattern of template]
+    // CHECK-DEFINE-PATTERN: [name of describing template][list of parameters]: [pattern of template]
+
----------------
Please summarize the new syntax in the paragraph above, and use small examples here instead.

================
Comment at: test/FileCheck/check-pattern.txt:39
@@ +38,1 @@
+// CHECK: test.cpp:[[@LINE]]:6: error: expected ';' after top level declarator
\ No newline at end of file

----------------
Add tests for:

  # All error messages you've introduced
  # Allow -check-prefix=FOO, FOO-DEFINE-PATTERN
  # Disallow BAR-DEFINE-PATTERN and FOO-DEFINE-PATTERN for the same definition name
  # Capture '@LINE' in a variable
  # Disallow using a pattern captured with a FOO-definition in a BAR check
  # Verify CHECK-{NEXT, SAME, DAG} etc. work with captured patterns


https://reviews.llvm.org/D22403