[PATCH] D60387: FileCheck [7/12]: Arbitrary long numeric expressions

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:32
+
+StringRef toString(const std::unordered_set<std::string> &Set) {
+  bool First = true;
----------------
static?

Though saying that, this is inside an anonymous namespace, so perhaps the other functions need static removing.

This function returns a `StringRef`, yet the base type is a local variable. I think that will result in the memory being freed under the reference. Perhaps better would be to make Str a Twine and then use Twine::str() to return a std::string.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:38
+    if (!First)
+      Str += " ";
+    First = false;
----------------
Perhaps make this ", ", and start and end the full string with '{'/'}' to make it easier to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60387