[PATCH] D64231: [FileCheck] Simplify numeric variable interface

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:74
 
-  /// Sets value of this numeric variable if not defined. \returns whether the
-  /// variable was already defined.
-  bool setValue(uint64_t Value);
+  /// Sets value of this yet undefined numeric variable.
+  void setValue(uint64_t Value);
----------------
This phrasing feels a bit weird to me. How about:

"Sets value of this numeric variable, if undefined. The function will trigger an assertion failure if the variable is actually defined."


================
Comment at: llvm/include/llvm/Support/FileCheck.h:77
 
-  /// Clears value of this numeric variable. \returns whether the variable was
-  /// already undefined.
-  bool clearValue();
+  /// Clears value of this numeric variable, irregardless of whether it is
+  /// currently defined or not.
----------------
irregardless -> regardless


================
Comment at: llvm/lib/Support/FileCheck.cpp:28
+void FileCheckNumericVariable::setValue(uint64_t NewValue) {
+  assert(!Value && "Overwriting numeric variable's value");
   Value = NewValue;
----------------
perhaps add "is not allowed".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64231