[PATCH] D77383: [FileCheck] Allow parenthesized expressions

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with a few nits and once thopre's comments have been addressed. Approving now as I'm off tomorrow, so if you get them done before I'm back, I don't want to hold things up.



================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:697
   before, after and between any of these elements.
+  There is currently no support for operator precendence, however, parentheses
+  can be used to change the evaluation order.
----------------
This sentence reads slightly clunkily to me. How about "however, parentheses" -> "but"?


================
Comment at: llvm/lib/Support/FileCheckImpl.h:668
   /// AST of the expression or an error holding a diagnostic against \p SM
-  /// otherwise.
+  /// otherwise. If @p Expr starts with a "(" this function will attempt to
+  /// parse a parenthesized expression.
----------------
Shouldn't it be `\p Expr`?


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:755-756
+
+  // Test more closing than opening parentheses. The diagnostic messages are
+  // not ideal, but for now simply check that we reject invalid input.
+  expectDiagnosticError("invalid operand format ')'",
----------------
If you plan on improving the situation, perhaps move the second sentence to a TODO comment on the next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77383