[PATCH] D62146: FileCheck: Improve FileCheck variable terminology

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

There's quite a bit more functional changes here than I was expecting. I was expecting this to be a pure renaming exercise.



================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:503
+FileCheck Regex Matching Syntax
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
----------------
This line needs shortening, or the docs will probably fail to build.


================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:594
+Unlike string substitution blocks, numeric substitution blocks only introduce
+numeric substitutions which substitution a numeric expression for its value.
 For example:
----------------
which substitution -> which substitute


================
Comment at: llvm/include/llvm/Support/FileCheck.h:106
 
 class FileCheckPatternContext;
 
----------------
Does FileCheckPatternContext still make sense as a name?


================
Comment at: llvm/include/llvm/Support/FileCheck.h:108-109
 
-/// Class representing a substitution to perform in the string to match.
-class FileCheckPatternSubstitution {
-private:
-  /// Pointer to a class instance holding the table with the values of live
-  /// pattern variables at the start of any given CHECK line. Used for
-  /// substituting pattern variables (numeric variables have their value in the
-  /// FileCheckNumExpr class instance pointed to by NumExpr).
+/// Class representing a substitution to perform in the RegExStr string to
+/// match.
+class FileCheckSubstitution {
----------------
I don't think you need "to match" here, if you are naming the variable directly.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:111
+class FileCheckSubstitution {
+protected:
+  /// Pointer to a class instance holding, among other things, the table with
----------------
This isn't just a naming change here...!


================
Comment at: llvm/include/llvm/Support/FileCheck.h:114
+  /// the values of live string variables at the start of any given CHECK line.
+  /// Used for substituting string variables for the text they were defined to.
+  /// Numeric expression are linked to the numeric variables they use at parse
----------------
for the text -> with the text
defined to -> defined as


================
Comment at: llvm/include/llvm/Support/FileCheck.h:115
+  /// Used for substituting string variables for the text they were defined to.
+  /// Numeric expression are linked to the numeric variables they use at parse
+  /// time and access directly the value of the numeric variable to evaluate
----------------
expression -> expressions


================
Comment at: llvm/include/llvm/Support/FileCheck.h:116
+  /// Numeric expression are linked to the numeric variables they use at parse
+  /// time and access directly the value of the numeric variable to evaluate
+  /// their value.
----------------
access directly -> directly access


================
Comment at: llvm/include/llvm/Support/FileCheck.h:238
 class FileCheckPatternContext {
   friend class FileCheckPattern;
 
----------------
Does FileCheckPattern still make sense in the new naming scheme?


================
Comment at: llvm/lib/Support/FileCheck.cpp:314
       }
-      // Strip the subtitution we are parsing. End points to the start of the
-      // "]]" closing the expression so account for it in computing the index
-      // of the first unparsed character.
+      // Strip the subtitution block we are parsing. End points to the start of
+      // the "]]" closing the expression so account for it in computing the
----------------
subtituion -> substitution


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62146