[PATCH] D62146: FileCheck: Improve FileCheck variable terminology
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 22 06:02:17 PDT 2019
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
More information about the llvm-commits
mailing list