[PATCH] D81667: [RFC, FileCheck] Add precision to format specifier

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:742-743
+ ``<precision>`` indicates the minimum number of digits that the value matched
+ must have, expecting leading zeros to have that amount of digits should the
+ value be too small otherwise.
+
----------------
I think that is much simpler.


================
Comment at: llvm/lib/Support/FileCheck.cpp:47
+Expected<std::string> ExpressionFormat::getWildcardRegex() const {
+  auto CreatePrecisionRegex = [this->Precision](StringRef S) {
+    return (S + Twine(Precision) + "}").str();
----------------
This doesn't compile. I don't think you can use `->` in a capture list. You just need to specify `this` and then use appropriately below.


================
Comment at: llvm/test/FileCheck/numeric-expression.txt:147
 
+; Numeric expressions in explicit matching format with precision and default
+; matching rule using variables defined on other lines without spaces.
----------------
Same goes elsewhere.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:142
     SmallVector<StringRef, 4> Matches;
+    std::string ExtendedInput;
+    if (TrailExtendTo > Input.size()) {
----------------
I think you could simplify this code by starting with `std::string ExtendedInput = Input;` and then just using `ExtendedInput` in the checks below.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:153-162
+  void checkWildcardRegexMatchFailure(StringRef Input,
+                                      bool TestChars = false) const {
+    if (!TestChars) {
+      EXPECT_FALSE(WildcardRegex.match(Input));
+      return;
+    }
+
----------------
It sounds to me like this is really just two completely different functions. I'd recommend splitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81667