[PATCH] D62146: FileCheck: Improve FileCheck variable terminology

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

A few more phrasing issues, otherwise LGTM.



================
Comment at: llvm/include/llvm/Support/FileCheck.h:262
+  /// sign), i.e. removes them from GlobalVariableTable and
+  /// GlobalNumericVariableTable and for numeric variable clear their value as
+  /// well.
----------------
Is the i.e. referring to just the two Global* variables, or is the "and ..." included?

Also:
variable -> variables
clear -> clears
I'd probably change:
"for numeric variable clear their value as well" to "also clears the value of numeric variables".


================
Comment at: llvm/lib/Support/FileCheck.cpp:387
+          // Handle substitution of string variables ([[<var>]]) defined in
+          // previous CHECK pattern or substitution of a numeric expression.
+          FileCheckSubstitution Substitution =
----------------
pattern or substitution -> patterns or subsitutions


================
Comment at: llvm/lib/Support/FileCheck.cpp:470
+    // Substitute all string variables and numeric expressions whose value is
+    // only known now. Use of string variables defined on the same line are
     // handled by back-references.
----------------
only known now -> only now known (this sounds clearer to me)


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:231
 
-  // Substitution of undefined pattern variable fails.
-  FileCheckPatternSubstitution PatternSubstitution =
-      FileCheckPatternSubstitution(&Context, "VAR404", 42);
-  EXPECT_FALSE(PatternSubstitution.getResult());
+  // Substitution of undefined string variable fails.
+  FileCheckSubstitution Substitution =
----------------
of undefined -> of an undefined


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62146