[PATCH] D60385: FileCheck [5/12]: Introduce regular numeric variables

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:252
+  /// for a given variable is recorded in this table.
+  StringMap<std::shared_ptr<FileCheckNumExprVar>> GlobalNumericVariableTable;
+
----------------
arsenm wrote:
> unique_ptr?
No because numeric variables are also referenced by FileCheckASTBinop and when parsing a numeric variable redefinition the variable stops being pointed at by GlobalNumericVariableTable but might still be referenced by a FileCheckASTBinop instance. Take for instance the following case:

CHECK: [[#@LINE]]
CHECK: [[#@LINE]]

A first numeric variable instance is created for the first @LINE and when the second CHECK is parsed by ParsePattern a new instance is created. At that point we want the first instance to still be live since it will be accessed when matching the first line letter.


================
Comment at: llvm/lib/Support/FileCheck.cpp:27-51
+/// Return value of this numeric variable.
+llvm::Optional<uint64_t> FileCheckNumExprVar::getValue() const {
+  if (!Defined)
+    return llvm::None;
+  return Value;
+}
+
----------------
arsenm wrote:
> This can go away if you just use Optional as the member
Sorry I don't follow. An Optional variable as the member of the binop? How does that help? I still have to check whether that variable has a value or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60385