[PATCH] D60389: FileCheck [9/12]: Add support for matching formats

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 05:20:43 PDT 2019


grimar added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:40
+    return StringRef("[[:digit:]]+");
+}
+
----------------
You can simplify this:


```
if (!Hex)
  return StringRef("[[:digit:]]+");
if (Cap)
  return StringRef("[[:digit:]A-F]+");
return StringRef("[[:digit:]a-f]+");
```


================
Comment at: llvm/lib/Support/FileCheck.cpp:48
+    return utostr(Value);
+}
+
----------------
You don't need `else` after `return`:

```
  if (Hex)
    return utohexstr(Value, !Cap);
  return utostr(Value);
```

(and in other methods)


================
Comment at: llvm/lib/Support/FileCheck.cpp:91
+
+  return NumExpr->getEffectiveFmt();
 }
----------------
`return NumExpr ? NumExpr->getEffectiveFmt() : FmtNone;`?


================
Comment at: llvm/lib/Support/FileCheck.cpp:225
+    return true;
+  return false;
+}
----------------
Just `return !S.consume_front(SkipStr) && !Optional`?


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:399
   // right value, getUndefVarNames does not return any variable.
+  auto DefNumExpr = FileCheckNumExpr(nullptr, FmtUnsigned);
   auto LineVar =
----------------
There is a good practive to avoid using `auto` in case the type isn't obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60389





More information about the llvm-commits mailing list