[PATCH] D60386: FileCheck [6/12]: Introduce numeric variable definition

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:249
+  /// defined.
+  std::shared_ptr<FileCheckNumExprVar> NumVarDef;
+
----------------
Why shared_ptr and not unique_ptr?


================
Comment at: llvm/lib/Support/FileCheck.cpp:198-199
+
+    return false;
+  } else {
+    // This method is indirectly called from ParsePattern for all numeric
----------------
No else after return


================
Comment at: llvm/lib/Support/FileCheck.cpp:296
+    const SourceMgr &SM) const {
+  std::shared_ptr<FileCheckASTBinop> NumExprAST;
+
----------------
unique_ptr


================
Comment at: llvm/lib/Support/FileCheck.cpp:307-309
+    if (parseNumericVariable(DefExpr, Name, true /*IsDefinition*/, SM))
+      // Invalid variable definition. Error reporting done in parsing function.
+      return nullptr;
----------------
Braces


================
Comment at: llvm/lib/Support/FileCheck.cpp:463-464
+      unsigned SubstInsertIdx = RegExStr.size();
+      std::shared_ptr<FileCheckNumExprVar> NumVarDef;
+      std::shared_ptr<FileCheckNumExpr> NumExpr;
+
----------------
unique_ptr


================
Comment at: llvm/test/FileCheck/numeric-expression.txt:32-33
+; CHECK-NEXT: [[# VAR1c : ]]
+
+
+; Numeric expressions using variables defined on other lines without spaces
----------------
Should also check with 0, and a count that exceeds the number of lines in the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60386