[PATCH] D60386: FileCheck [6/12]: Introduce numeric variable definition

Tue Jun 4 09:59:03 PDT 2019

thopre added a comment.

In D60386#1529382 <https://reviews.llvm.org/D60386#1529382>, @probinson wrote:

> Grammar nits and one longer point, which is:
>  I understand where the "parenthesis group" term came from, but I think it's not appropriate here.  While a CHECK line is implicitly a regex, and variables are a kind of back-reference, the syntax for variable references (use or def) is not parentheses and the content is not a "group" in any real sense, and so "parenthesis group" is an un-obvious and confusing term.
>  Because FileCheck uses square brackets, I propose that the least disruptive change at this point would be to call them "bracket expressions." The "bracket" part is obvious, and "expression" because (a) definitions will have a regex, and (b) numeric variable references are implicitly expressions.  (Okay, still a bit of a stretch, but less so that "parenthesis group" IMO.)
>  WDYT?

Actually the parenthesis group refers to the regex being generated for a given CHECK line and fed to the regex engine. Let me give an example:

CHECK: foo [[STRVAR:([a-z]+)]] #NUMVAR:

would create the regex "foo (([a-z]+)) ([0-9]+)" and record that the value for STRVAL is captured by the parenthesis group 1 (0 captures what the whole regex matches) and NUMVAR's value is captured by the parenthesis group 3. The Regex::match function takes an array where each entry takes the matched text of the corresponding parenthesis group. The FileCheckNumExprMatch structure records for numeric variable definitions what entries to look for the values in that array so parenthesis group is perfectly adequate (apart from perhaps better naming) IMHO.

> Also let me apologize in advance for the review sparsity lately, and going forward.  I was at a conference all last week, and I will be on holiday with limited internet through the end of next week.

Sparsity of review is a given when reviewer is from a different company, I adapt my workflow around it. Likewise I try to respond to comments as fast as possible but might not always be able to.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60386