[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