[PATCH] D62241: [FileCheck] Introduce substitution subclasses

[James Henderson via Phabricator via llvm-commits]

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

LGTM, with two small comments. Might be best getting @probinson to give a final thumbs up though, since he suggested it.



================
Comment at: llvm/include/llvm/Support/FileCheck.h:141-143
   /// instance or None if substitution failed. Numeric expressions are
   /// substituted by their values. String variables are simply replaced by the
   /// text their definition matched.
----------------
The explanations of the different substitutions performed (i.e. the second and third sentences of this comment) don't belong in this class here. I'd just remove them.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:177-178
+
+  /// \returns the result of evaluating the numeric expression in this
+  /// substitution as a string, or None if evaluation failed.
   llvm::Optional<std::string> getResult() const;
----------------
Maybe better to rephrase this as "\returns a string containing the result...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62241