[PATCH] D104572: [lit] Add the ability to parse regexes in Lit boolean expressions

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added a comment.

This seems like a worthwhile feature.  Thanks for working on it.  It's not a part of lit I have much experience with, so hopefully someone else will comment too.

I've commented some on the implementation.  I haven't looked at the tests much yet.



================
Comment at: llvm/utils/lit/lit/BooleanExpression.py:18
     #   identifier :: [-+=._a-zA-Z0-9]+
+    #   regex      :: '{{' any-regex '}}'
 
----------------
I think the following renaming would make this easier to understand:

* `regex` -> `braced_regex`: It's not just a plain regular expression.
* `any-regex` -> `python_regex`:  It's written in python's regular expression language.


================
Comment at: llvm/utils/lit/lit/BooleanExpression.py:114
             self.expect(')')
         elif not BooleanExpression.isIdentifier(self.token):
             raise ValueError("expected: '!' or '(' or identifier\nhave: %s" %
----------------
`isIdentifier` seems misnamed if it accepts `{{`.

The logic should probably be something like either `not isIdentifier and not isRegexOpen` or `not isIdentiferOrRegexOpen`.


================
Comment at: llvm/utils/lit/lit/BooleanExpression.py:115
         elif not BooleanExpression.isIdentifier(self.token):
             raise ValueError("expected: '!' or '(' or identifier\nhave: %s" %
                              self.quote(self.token))
----------------
Shouldn't this mention `{{` as a possibility?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104572