[PATCH] D49084: FileCheck: Add support for numeric variables and expressions

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

Thanks for updating this patch. It will be very useful!



================
Comment at: include/llvm/Support/FileCheck.h:225
+  /// Pointer to AST of the numeric expression.
+  std::shared_ptr<FileCheckNumExprAST> AST;
+
----------------
Does this need to be a `shared_ptr`? Could it be a `unique_ptr` instead?


================
Comment at: lib/Support/FileCheck.cpp:81
+/// representation (true if value is invalid).
+bool FileCheckNumExprVal::GetStringRepr(struct FileCheckNumExprFmtType Fmt,
+                                        std::string &StringRepr) const {
----------------
Could this return a `llvm::Optional<std::string>` to avoid the output-parameter?


================
Comment at: lib/Support/FileCheck.cpp:1076
+        } else
+          CaptureParenNeeded = false;
+
----------------
This could be deleted if you initialize `CaptureParenNeeded` to false at the beginning of the scope. It would also ensure that the variable is never uninitialized.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D49084