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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:70
+    WildcardRegex << Precision << '}';
+    return WildcardRegex.str();
+  }
----------------
Seems you should be able to do the following instead?

```
return (RegexPrefix + Twine(Precision) + "}").str();
```


================
Comment at: llvm/lib/Support/FileCheck.cpp:86
   }
 }
 
----------------
Perhaps, it might be simpler just to merge switches and write the logic here as:


```
Expected<std::string> ExpressionFormat::getWildcardRegex() const {
  if (Value == Kind::NoFormat)
    return createStringError(std::errc::invalid_argument,
                             "trying to match value with invalid format");
  switch (Value) {
  case Kind::Unsigned:
    if (Precision)
      return ("-?([1-9][0-9]*)?[0-9]{" + Twine(Precision) + "}").str();
    return std::string("[0-9]+");
  case Kind::Signed:
     ...
  default:
    llvm_unreachable("....");
  }
}
```


================
Comment at: llvm/lib/Support/FileCheck.cpp:780
+    FormatExpr = FormatExpr.ltrim(SpaceChars);
+    FormatExpr = FormatExpr.rtrim(SpaceChars);
+    if (!FormatExpr.consume_front("%"))
----------------
Use `trim`?

```
FormatExpr.trim(SpaceChars)
```


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:165
+  std::string padWithLeadingZeros(StringRef NumStr) const {
+    bool Negative = NumStr.front() == '-';
+    if (NumStr.size() - unsigned(Negative) >= Precision)
----------------
This will fail if `NumStr` is empty. Is it OK (I guess so), though perhaps a bit cleaner would be to use `StringRef::startswith`.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:171
+    if (Negative) {
+      PaddedStr.push_back('-');
+      NumStr = NumStr.drop_front();
----------------
```
PaddedStr = "-";
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81667