[clang] ce2b5cb - [libTooling] Simplify type structure of `Stencil`s.

Yitzhak Mandelbaum via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 6 07:42:26 PST 2019


Author: Yitzhak Mandelbaum
Date: 2019-11-06T10:28:34-05:00
New Revision: ce2b5cb6decb5cce32adde0d23bdea521da0908b

URL: https://github.com/llvm/llvm-project/commit/ce2b5cb6decb5cce32adde0d23bdea521da0908b
DIFF: https://github.com/llvm/llvm-project/commit/ce2b5cb6decb5cce32adde0d23bdea521da0908b.diff

LOG: [libTooling] Simplify type structure of `Stencil`s.

Summary:
Currently, stencils are defined as a sequence of `StencilParts`. This
differentiation adds an unneeded layer of complexity to the definition of
Stencils. This change significantly simplifies the type structure: a stencil is
now conceptually any object implementing `StencilInterface` and `Stencil` is
just a thin wrapper for pointers to this interface.

To account for the sequencing that was supported by the old `Stencil` type, we
introduce a sequencing class that implements `StencilInterface`. That is,
sequences are just another kind of Stencil and no longer have any special
status.

Corresponding to this change in the type structure, we change the way `cat` is
used (and defined). `cat` bundles multiple features: it builds a stencil from a
sequence of subcomponents and admits multiple different types for its arguments,
while coercing them into the right type. Previously, `cat` was also used to
coerce a single `StencilPart` into a `Stencil`. With that distinction gone, many
uses of `cat` (e.g. in the tests) are unnecessary and have, therefore, been
removed.

Reviewers: gribozavr

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69613

Added: 
    

Modified: 
    clang/include/clang/Tooling/Transformer/Stencil.h
    clang/lib/Tooling/Transformer/Stencil.cpp
    clang/unittests/Tooling/StencilTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Transformer/Stencil.h b/clang/include/clang/Tooling/Transformer/Stencil.h
index 6ef44e5ce7ba..ca23a6355f81 100644
--- a/clang/include/clang/Tooling/Transformer/Stencil.h
+++ b/clang/include/clang/Tooling/Transformer/Stencil.h
@@ -32,197 +32,132 @@
 
 namespace clang {
 namespace transformer {
-/// A stencil is represented as a sequence of "parts" that can each individually
-/// generate a code string based on a match result.  The 
diff erent kinds of
-/// parts include (raw) text, references to bound nodes and assorted operations
-/// on bound nodes.
-///
-/// Users can create custom Stencil operations by implementing this interface.
-class StencilPartInterface {
+class StencilInterface {
 public:
-  virtual ~StencilPartInterface() = default;
+  virtual ~StencilInterface() = default;
 
   /// Evaluates this part to a string and appends it to \c Result.  \c Result is
-  /// undefined in the case of an error.
+  /// undefined in the case of an error. `Result` is an out parameter to
+  /// optimize the expected common case of evaluating a sequence of generators.
   virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match,
                            std::string *Result) const = 0;
 
-  /// Constructs a string representation of the StencilPart. StencilParts
-  /// generated by the `selection` and `run` functions do not have a unique
-  /// string representation.
+  /// Convenience version of `eval`, for the case where the generator is being
+  /// evaluated on its own.
+  llvm::Expected<std::string>
+  eval(const ast_matchers::MatchFinder::MatchResult &R) const;
+
+  /// Constructs a string representation of the StencilInterface. The
+  /// representation must be deterministic, but is not required to be unique.
   virtual std::string toString() const = 0;
 
 protected:
-  StencilPartInterface() = default;
+  StencilInterface() = default;
 
   // Since this is an abstract class, copying/assigning only make sense for
   // derived classes implementing `clone()`.
-  StencilPartInterface(const StencilPartInterface &) = default;
-  StencilPartInterface &operator=(const StencilPartInterface &) = default;
-};
-
-/// A copyable facade for a std::unique_ptr<StencilPartInterface>. Copies result
-/// in a copy of the underlying pointee object.
-class StencilPart {
-public:
-  explicit StencilPart(std::shared_ptr<StencilPartInterface> Impl)
-      : Impl(std::move(Impl)) {}
-
-  /// See `StencilPartInterface::eval()`.
-  llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match,
-                   std::string *Result) const {
-    return Impl->eval(Match, Result);
-  }
-
-  std::string toString() const {
-    if (Impl == nullptr)
-      return "";
-    return Impl->toString();
-  }
-
-private:
-  std::shared_ptr<StencilPartInterface> Impl;
+  StencilInterface(const StencilInterface &) = default;
+  StencilInterface &operator=(const StencilInterface &) = default;
 };
 
 /// A sequence of code fragments, references to parameters and code-generation
-/// operations that together can be evaluated to (a fragment of) source code,
-/// given a match result.
+/// operations that together can be evaluated to (a fragment of) source code or
+/// a diagnostic message, given a match result.
+///
+/// We use a `shared_ptr` to allow for easy and cheap copying of stencils.
+/// Since `StencilInterface` is an immutable interface, the sharing doesn't
+/// impose any risks. Otherwise, we would have to add a virtual `copy` method to
+/// the API and implement it for all derived classes.
 class Stencil {
+  std::shared_ptr<StencilInterface> Impl;
+
 public:
   Stencil() = default;
-  Stencil(std::vector<StencilPart> Parts) : Parts(std::move(Parts)) {}
-
-  /// Composes a stencil from a series of parts.
-  template <typename... Ts> static Stencil cat(Ts &&... Parts) {
-    Stencil S;
-    S.Parts = {wrap(std::forward<Ts>(Parts))...};
-    return S;
-  }
+  explicit Stencil(std::shared_ptr<StencilInterface> Ptr)
+      : Impl(std::move(Ptr)) {}
 
-  /// Appends data from a \p OtherStencil to this stencil.
-  void append(Stencil OtherStencil);
+  StencilInterface *operator->() const { return Impl.get(); }
 
-  // Evaluates the stencil given a match result. Requires that the nodes in the
-  // result includes any ids referenced in the stencil. References to missing
-  // nodes will result in an invalid_argument error.
   llvm::Expected<std::string>
-  eval(const ast_matchers::MatchFinder::MatchResult &Match) const;
-
-  // Allow Stencils to operate as std::function, for compatibility with
-  // Transformer's TextGenerator.
-  llvm::Expected<std::string>
-  operator()(const ast_matchers::MatchFinder::MatchResult &Result) const {
-    return eval(Result);
-  }
-
-  /// Constructs a string representation of the Stencil. The string is not
-  /// guaranteed to be unique.
-  std::string toString() const {
-    std::vector<std::string> PartStrings;
-    PartStrings.reserve(Parts.size());
-    for (const auto &Part : Parts)
-      PartStrings.push_back(Part.toString());
-    return llvm::join(PartStrings, ", ");
+  operator()(const ast_matchers::MatchFinder::MatchResult &R) {
+    return Impl->eval(R);
   }
-
-private:
-  static StencilPart wrap(llvm::StringRef Text);
-  static StencilPart wrap(RangeSelector Selector);
-  static StencilPart wrap(StencilPart Part) { return Part; }
-
-  std::vector<StencilPart> Parts;
 };
 
+namespace detail {
+/// Convenience function to construct a \c Stencil. Overloaded for common cases
+/// so that user doesn't need to specify which factory function to use. This
+/// pattern gives benefits similar to implicit constructors, while maintaing a
+/// higher degree of explictness.
+Stencil makeStencil(llvm::StringRef Text);
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
+} // namespace detail
+
+/// Constructs the string representing the concatenation of the given \p
+/// Parts. If only one element is passed in \p Parts, returns that element.
+Stencil catVector(std::vector<Stencil> Parts);
+
+/// Concatenates 0+ stencil pieces into a single stencil. Arguments can be raw
+/// text, ranges in the matched code (\p RangeSelector) or other `Stencil`s.
+template <typename... Ts> Stencil cat(Ts &&... Parts) {
+  return catVector({detail::makeStencil(std::forward<Ts>(Parts))...});
+}
+
 //
 // Functions for conveniently building stencils.
 //
 
-/// Convenience wrapper for Stencil::cat that can be imported with a using decl.
-template <typename... Ts> Stencil cat(Ts &&... Parts) {
-  return Stencil::cat(std::forward<Ts>(Parts)...);
-}
-/// Convenience wrapper for Stencil constructor of the same type. Declaration
-/// outside of the class supports transition of `Stencil` type to an alias
-/// rather than a class.
-inline Stencil catVector(std::vector<StencilPart> Parts) {
-  return Stencil(std::move(Parts));
-}
-
 /// \returns exactly the text provided.
-StencilPart text(llvm::StringRef Text);
+Stencil text(llvm::StringRef Text);
 
 /// \returns the source corresponding to the selected range.
-StencilPart selection(RangeSelector Selector);
+Stencil selection(RangeSelector Selector);
 
 /// Generates the source of the expression bound to \p Id, wrapping it in
 /// parentheses if it may parse 
diff erently depending on context. For example, a
 /// binary operation is always wrapped, while a variable reference is never
 /// wrapped.
-StencilPart expression(llvm::StringRef Id);
+Stencil expression(llvm::StringRef Id);
 
 /// Constructs an idiomatic dereferencing of the expression bound to \p ExprId.
 /// \p ExprId is wrapped in parentheses, if needed.
-StencilPart deref(llvm::StringRef ExprId);
+Stencil deref(llvm::StringRef ExprId);
 
 /// Constructs an expression that idiomatically takes the address of the
 /// expression bound to \p ExprId. \p ExprId is wrapped in parentheses, if
 /// needed.
-StencilPart addressOf(llvm::StringRef ExprId);
+Stencil addressOf(llvm::StringRef ExprId);
 
 /// Constructs a `MemberExpr` that accesses the named member (\p Member) of the
 /// object bound to \p BaseId. The access is constructed idiomatically: if \p
 /// BaseId is bound to `e` and \p Member identifies member `m`, then returns
 /// `e->m`, when e is a pointer, `e2->m` when e = `*e2` and `e.m` otherwise.
 /// Additionally, `e` is wrapped in parentheses, if needed.
-StencilPart access(llvm::StringRef BaseId, StencilPart Member);
-inline StencilPart access(llvm::StringRef BaseId, llvm::StringRef Member) {
+Stencil access(llvm::StringRef BaseId, Stencil Member);
+inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
   return access(BaseId, text(Member));
 }
 
 /// Chooses between the two stencil parts, based on whether \p ID is bound in
 /// the match.
-StencilPart ifBound(llvm::StringRef Id, StencilPart TruePart,
-                    StencilPart FalsePart);
+Stencil ifBound(llvm::StringRef Id, Stencil TrueStencil, Stencil FalseStencil);
 
 /// Chooses between the two strings, based on whether \p ID is bound in the
 /// match.
-inline StencilPart ifBound(llvm::StringRef Id, llvm::StringRef TrueText,
-                           llvm::StringRef FalseText) {
+inline Stencil ifBound(llvm::StringRef Id, llvm::StringRef TrueText,
+                       llvm::StringRef FalseText) {
   return ifBound(Id, text(TrueText), text(FalseText));
 }
 
-/// Wraps a MatchConsumer in a StencilPart, so that it can be used in a Stencil.
-/// This supports user-defined extensions to the Stencil language.
-StencilPart run(MatchConsumer<std::string> C);
+/// Wraps a \c MatchConsumer in a \c Stencil, so that it can be used in a \c
+/// Stencil.  This supports user-defined extensions to the \c Stencil language.
+Stencil run(MatchConsumer<std::string> C);
 
 /// For debug use only; semantics are not guaranteed.
 ///
 /// \returns the string resulting from calling the node's print() method.
-StencilPart dPrint(llvm::StringRef Id);
+Stencil dPrint(llvm::StringRef Id);
 } // namespace transformer
-
-namespace tooling {
-// DEPRECATED: These are temporary aliases supporting client migration to the
-// `transformer` namespace.
-using Stencil = transformer::Stencil;
-using StencilPart = transformer::StencilPart;
-namespace stencil {
-using transformer::access;
-using transformer::addressOf;
-using transformer::cat;
-using transformer::deref;
-using transformer::dPrint;
-using transformer::expression;
-using transformer::ifBound;
-using transformer::run;
-using transformer::selection;
-using transformer::text;
-/// \returns the source corresponding to the identified node.
-/// FIXME: Deprecated. Write `selection(node(Id))` instead.
-inline transformer::StencilPart node(llvm::StringRef Id) {
-  return selection(tooling::node(Id));
-}
-} // namespace stencil
-} // namespace tooling
 } // namespace clang
 #endif // LLVM_CLANG_TOOLING_TRANSFORMER_STENCIL_H_

diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp b/clang/lib/Tooling/Transformer/Stencil.cpp
index 984950a54e96..d994fdcf8707 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -15,6 +15,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Transformer/SourceCode.h"
 #include "clang/Tooling/Transformer/SourceCodeBuilders.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Errc.h"
 #include <atomic>
@@ -77,19 +78,24 @@ struct SelectorData {
 
 // A stencil operation to build a member access `e.m` or `e->m`, as appropriate.
 struct AccessData {
-  AccessData(StringRef BaseId, StencilPart Member)
+  AccessData(StringRef BaseId, Stencil Member)
       : BaseId(BaseId), Member(std::move(Member)) {}
   std::string BaseId;
-  StencilPart Member;
+  Stencil Member;
 };
 
 struct IfBoundData {
-  IfBoundData(StringRef Id, StencilPart TruePart, StencilPart FalsePart)
-      : Id(Id), TruePart(std::move(TruePart)), FalsePart(std::move(FalsePart)) {
-  }
+  IfBoundData(StringRef Id, Stencil TrueStencil, Stencil FalseStencil)
+      : Id(Id), TrueStencil(std::move(TrueStencil)),
+        FalseStencil(std::move(FalseStencil)) {}
   std::string Id;
-  StencilPart TruePart;
-  StencilPart FalsePart;
+  Stencil TrueStencil;
+  Stencil FalseStencil;
+};
+
+struct SequenceData {
+  SequenceData(std::vector<Stencil> Stencils) : Stencils(std::move(Stencils)) {}
+  std::vector<Stencil> Stencils;
 };
 
 std::string toStringData(const RawTextData &Data) {
@@ -126,13 +132,14 @@ std::string toStringData(const SelectorData &) { return "selection(...)"; }
 
 std::string toStringData(const AccessData &Data) {
   return (llvm::Twine("access(\"") + Data.BaseId + "\", " +
-          Data.Member.toString() + ")")
+          Data.Member->toString() + ")")
       .str();
 }
 
 std::string toStringData(const IfBoundData &Data) {
   return (llvm::Twine("ifBound(\"") + Data.Id + "\", " +
-          Data.TruePart.toString() + ", " + Data.FalsePart.toString() + ")")
+          Data.TrueStencil->toString() + ", " + Data.FalseStencil->toString() +
+          ")")
       .str();
 }
 
@@ -140,6 +147,14 @@ std::string toStringData(const MatchConsumer<std::string> &) {
   return "run(...)";
 }
 
+std::string toStringData(const SequenceData &Data) {
+  llvm::SmallVector<std::string, 2> Parts;
+  Parts.reserve(Data.Stencils.size());
+  for (const auto &S : Data.Stencils)
+    Parts.push_back(S->toString());
+  return (llvm::Twine("seq(") + llvm::join(Parts, ", ") + ")").str();
+}
+
 // The `evalData()` overloads evaluate the given stencil data to a string, given
 // the match result, and append it to `Result`. We define an overload for each
 // type of stencil data.
@@ -214,14 +229,14 @@ Error evalData(const AccessData &Data, const MatchFinder::MatchResult &Match,
           errc::invalid_argument,
           "Could not construct object text from ID: " + Data.BaseId);
   }
-  return Data.Member.eval(Match, Result);
+  return Data.Member->eval(Match, Result);
 }
 
 Error evalData(const IfBoundData &Data, const MatchFinder::MatchResult &Match,
                std::string *Result) {
   auto &M = Match.Nodes.getMap();
-  return (M.find(Data.Id) != M.end() ? Data.TruePart : Data.FalsePart)
-      .eval(Match, Result);
+  return (M.find(Data.Id) != M.end() ? Data.TrueStencil : Data.FalseStencil)
+      ->eval(Match, Result);
 }
 
 Error evalData(const MatchConsumer<std::string> &Fn,
@@ -233,13 +248,20 @@ Error evalData(const MatchConsumer<std::string> &Fn,
   return Error::success();
 }
 
-template <typename T>
-class StencilPartImpl : public StencilPartInterface {
+Error evalData(const SequenceData &Data, const MatchFinder::MatchResult &Match,
+               std::string *Result) {
+  for (const auto &S : Data.Stencils)
+    if (auto Err = S->eval(Match, Result))
+      return Err;
+  return Error::success();
+}
+
+template <typename T> class StencilImpl : public StencilInterface {
   T Data;
 
 public:
   template <typename... Ps>
-  explicit StencilPartImpl(Ps &&... Args) : Data(std::forward<Ps>(Args)...) {}
+  explicit StencilImpl(Ps &&... Args) : Data(std::forward<Ps>(Args)...) {}
 
   Error eval(const MatchFinder::MatchResult &Match,
              std::string *Result) const override {
@@ -250,69 +272,67 @@ class StencilPartImpl : public StencilPartInterface {
 };
 } // namespace
 
-StencilPart Stencil::wrap(StringRef Text) {
-  return transformer::text(Text);
-}
-
-StencilPart Stencil::wrap(RangeSelector Selector) {
-  return transformer::selection(std::move(Selector));
+llvm::Expected<std::string>
+StencilInterface::eval(const MatchFinder::MatchResult &R) const {
+  std::string Output;
+  if (auto Err = eval(R, &Output))
+    return std::move(Err);
+  return Output;
 }
 
-void Stencil::append(Stencil OtherStencil) {
-  for (auto &Part : OtherStencil.Parts)
-    Parts.push_back(std::move(Part));
-}
+Stencil transformer::detail::makeStencil(StringRef Text) { return text(Text); }
 
-llvm::Expected<std::string>
-Stencil::eval(const MatchFinder::MatchResult &Match) const {
-  std::string Result;
-  for (const auto &Part : Parts)
-    if (auto Err = Part.eval(Match, &Result))
-      return std::move(Err);
-  return Result;
+Stencil transformer::detail::makeStencil(RangeSelector Selector) {
+  return selection(std::move(Selector));
 }
 
-StencilPart transformer::text(StringRef Text) {
-  return StencilPart(std::make_shared<StencilPartImpl<RawTextData>>(Text));
+Stencil transformer::text(StringRef Text) {
+  return Stencil(std::make_shared<StencilImpl<RawTextData>>(Text));
 }
 
-StencilPart transformer::selection(RangeSelector Selector) {
-  return StencilPart(
-      std::make_shared<StencilPartImpl<SelectorData>>(std::move(Selector)));
+Stencil transformer::selection(RangeSelector Selector) {
+  return Stencil(
+      std::make_shared<StencilImpl<SelectorData>>(std::move(Selector)));
 }
 
-StencilPart transformer::dPrint(StringRef Id) {
-  return StencilPart(std::make_shared<StencilPartImpl<DebugPrintNodeData>>(Id));
+Stencil transformer::dPrint(StringRef Id) {
+  return Stencil(std::make_shared<StencilImpl<DebugPrintNodeData>>(Id));
 }
 
-StencilPart transformer::expression(llvm::StringRef Id) {
-  return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+Stencil transformer::expression(llvm::StringRef Id) {
+  return Stencil(std::make_shared<StencilImpl<UnaryOperationData>>(
       UnaryNodeOperator::Parens, Id));
 }
 
-StencilPart transformer::deref(llvm::StringRef ExprId) {
-  return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+Stencil transformer::deref(llvm::StringRef ExprId) {
+  return Stencil(std::make_shared<StencilImpl<UnaryOperationData>>(
       UnaryNodeOperator::Deref, ExprId));
 }
 
-StencilPart transformer::addressOf(llvm::StringRef ExprId) {
-  return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+Stencil transformer::addressOf(llvm::StringRef ExprId) {
+  return Stencil(std::make_shared<StencilImpl<UnaryOperationData>>(
       UnaryNodeOperator::Address, ExprId));
 }
 
-StencilPart transformer::access(StringRef BaseId, StencilPart Member) {
-  return StencilPart(
-      std::make_shared<StencilPartImpl<AccessData>>(BaseId, std::move(Member)));
+Stencil transformer::access(StringRef BaseId, Stencil Member) {
+  return Stencil(
+      std::make_shared<StencilImpl<AccessData>>(BaseId, std::move(Member)));
+}
+
+Stencil transformer::ifBound(StringRef Id, Stencil TrueStencil,
+                             Stencil FalseStencil) {
+  return Stencil(std::make_shared<StencilImpl<IfBoundData>>(
+      Id, std::move(TrueStencil), std::move(FalseStencil)));
 }
 
-StencilPart transformer::ifBound(StringRef Id, StencilPart TruePart,
-                             StencilPart FalsePart) {
-  return StencilPart(std::make_shared<StencilPartImpl<IfBoundData>>(
-      Id, std::move(TruePart), std::move(FalsePart)));
+Stencil transformer::run(MatchConsumer<std::string> Fn) {
+  return Stencil(
+      std::make_shared<StencilImpl<MatchConsumer<std::string>>>(std::move(Fn)));
 }
 
-StencilPart transformer::run(MatchConsumer<std::string> Fn) {
-  return StencilPart(
-      std::make_shared<StencilPartImpl<MatchConsumer<std::string>>>(
-          std::move(Fn)));
+Stencil transformer::catVector(std::vector<Stencil> Parts) {
+  // Only one argument, so don't wrap in sequence.
+  if (Parts.size() == 1)
+    return std::move(Parts[0]);
+  return Stencil(std::make_shared<StencilImpl<SequenceData>>(std::move(Parts)));
 }

diff  --git a/clang/unittests/Tooling/StencilTest.cpp b/clang/unittests/Tooling/StencilTest.cpp
index c62eae79d245..5bbdfea398ab 100644
--- a/clang/unittests/Tooling/StencilTest.cpp
+++ b/clang/unittests/Tooling/StencilTest.cpp
@@ -96,7 +96,7 @@ class StencilTest : public ::testing::Test {
                      .bind("expr")))
             .bind("stmt"));
     ASSERT_TRUE(StmtMatch);
-    if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+    if (auto ResultOrErr = Stencil->eval(StmtMatch->Result)) {
       ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
     } else {
       auto Err = llvm::handleErrors(ResultOrErr.takeError(),
@@ -131,11 +131,12 @@ TEST_F(StencilTest, SingleStatement) {
   // Invert the if-then-else.
   auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ",
                      statement(Then));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
                        HasValue("if (!true) return 0; else return 1;"));
 }
 
-TEST_F(StencilTest, SingleStatementCallOperator) {
+// Tests `stencil`.
+TEST_F(StencilTest, StencilFactoryFunction) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
     if (true)
@@ -148,9 +149,9 @@ TEST_F(StencilTest, SingleStatementCallOperator) {
                       hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else))));
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ",
-                  statement(Then));
-  EXPECT_THAT_EXPECTED(S(StmtMatch->Result),
+  auto Consumer = cat("if (!", node(Condition), ") ", statement(Else), " else ",
+                      statement(Then));
+  EXPECT_THAT_EXPECTED(Consumer(StmtMatch->Result),
                        HasValue("if (!true) return 0; else return 1;"));
 }
 
@@ -165,7 +166,7 @@ TEST_F(StencilTest, UnboundNode) {
                                              hasThen(stmt().bind("a2"))));
   ASSERT_TRUE(StmtMatch);
   auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
-  auto ResultOrErr = Stencil.eval(StmtMatch->Result);
+  auto ResultOrErr = Stencil->eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
       << "Expected unbound node, got " << *ResultOrErr;
 }
@@ -176,14 +177,14 @@ void testExpr(StringRef Id, StringRef Snippet, const Stencil &Stencil,
               StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue(Expected));
 }
 
 void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
                  testing::Matcher<std::string> MessageMatcher) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
                        Failed<StringError>(testing::Property(
                            &StringError::getMessage, MessageMatcher)));
 }
@@ -195,28 +196,28 @@ TEST_F(StencilTest, SelectionOp) {
 
 TEST_F(StencilTest, IfBoundOpBound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5");
+  testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
 }
 
 TEST_F(StencilTest, IfBoundOpUnbound) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
+  testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
 }
 
 TEST_F(StencilTest, ExpressionOpNoParens) {
   StringRef Id = "id";
-  testExpr(Id, "3;", cat(expression(Id)), "3");
+  testExpr(Id, "3;", expression(Id), "3");
 }
 
 // Don't parenthesize a parens expression.
 TEST_F(StencilTest, ExpressionOpNoParensParens) {
   StringRef Id = "id";
-  testExpr(Id, "(3);", cat(expression(Id)), "(3)");
+  testExpr(Id, "(3);", expression(Id), "(3)");
 }
 
 TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
   StringRef Id = "id";
-  testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)");
+  testExpr(Id, "3+4;", expression(Id), "(3+4)");
 }
 
 // `expression` shares code with other ops, so we get sufficient coverage of the
@@ -224,33 +225,33 @@ TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
 // tests should be added.
 TEST_F(StencilTest, ExpressionOpUnbound) {
   StringRef Id = "id";
-  testFailure(Id, "3;", cat(expression("ACACA")),
+  testFailure(Id, "3;", expression("ACACA"),
               AllOf(HasSubstr("ACACA"), HasSubstr("not bound")));
 }
 
 TEST_F(StencilTest, DerefPointer) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x;", cat(deref(Id)), "*x");
+  testExpr(Id, "int *x; x;", deref(Id), "*x");
 }
 
 TEST_F(StencilTest, DerefBinOp) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; x + 1;", cat(deref(Id)), "*(x + 1)");
+  testExpr(Id, "int *x; x + 1;", deref(Id), "*(x + 1)");
 }
 
 TEST_F(StencilTest, DerefAddressExpr) {
   StringRef Id = "id";
-  testExpr(Id, "int x; &x;", cat(deref(Id)), "x");
+  testExpr(Id, "int x; &x;", deref(Id), "x");
 }
 
 TEST_F(StencilTest, AddressOfValue) {
   StringRef Id = "id";
-  testExpr(Id, "int x; x;", cat(addressOf(Id)), "&x");
+  testExpr(Id, "int x; x;", addressOf(Id), "&x");
 }
 
 TEST_F(StencilTest, AddressOfDerefExpr) {
   StringRef Id = "id";
-  testExpr(Id, "int *x; *x;", cat(addressOf(Id)), "x");
+  testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
 TEST_F(StencilTest, AccessOpValue) {
@@ -259,7 +260,7 @@ TEST_F(StencilTest, AccessOpValue) {
     x;
   )cc";
   StringRef Id = "id";
-  testExpr(Id, Snippet, cat(access(Id, "field")), "x.field");
+  testExpr(Id, Snippet, access(Id, "field"), "x.field");
 }
 
 TEST_F(StencilTest, AccessOpValueExplicitText) {
@@ -268,7 +269,7 @@ TEST_F(StencilTest, AccessOpValueExplicitText) {
     x;
   )cc";
   StringRef Id = "id";
-  testExpr(Id, Snippet, cat(access(Id, text("field"))), "x.field");
+  testExpr(Id, Snippet, access(Id, text("field")), "x.field");
 }
 
 TEST_F(StencilTest, AccessOpValueAddress) {
@@ -277,7 +278,7 @@ TEST_F(StencilTest, AccessOpValueAddress) {
     &x;
   )cc";
   StringRef Id = "id";
-  testExpr(Id, Snippet, cat(access(Id, "field")), "x.field");
+  testExpr(Id, Snippet, access(Id, "field"), "x.field");
 }
 
 TEST_F(StencilTest, AccessOpPointer) {
@@ -286,7 +287,7 @@ TEST_F(StencilTest, AccessOpPointer) {
     x;
   )cc";
   StringRef Id = "id";
-  testExpr(Id, Snippet, cat(access(Id, "field")), "x->field");
+  testExpr(Id, Snippet, access(Id, "field"), "x->field");
 }
 
 TEST_F(StencilTest, AccessOpPointerDereference) {
@@ -295,7 +296,7 @@ TEST_F(StencilTest, AccessOpPointerDereference) {
     *x;
   )cc";
   StringRef Id = "id";
-  testExpr(Id, Snippet, cat(access(Id, "field")), "x->field");
+  testExpr(Id, Snippet, access(Id, "field"), "x->field");
 }
 
 TEST_F(StencilTest, AccessOpExplicitThis) {
@@ -314,8 +315,8 @@ TEST_F(StencilTest, AccessOpExplicitThis) {
       matchStmt(Snippet, returnStmt(hasReturnValue(ignoringImplicit(memberExpr(
                              hasObjectExpression(expr().bind("obj")))))));
   ASSERT_TRUE(StmtMatch);
-  const Stencil Stencil = cat(access("obj", "field"));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+  const Stencil Stencil = access("obj", "field");
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result),
                        HasValue("this->field"));
 }
 
@@ -335,8 +336,8 @@ TEST_F(StencilTest, AccessOpImplicitThis) {
       matchStmt(Snippet, returnStmt(hasReturnValue(ignoringImplicit(memberExpr(
                              hasObjectExpression(expr().bind("obj")))))));
   ASSERT_TRUE(StmtMatch);
-  const Stencil Stencil = cat(access("obj", "field"));
-  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue("field"));
+  const Stencil Stencil = access("obj", "field");
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("field"));
 }
 
 TEST_F(StencilTest, RunOp) {
@@ -345,80 +346,106 @@ TEST_F(StencilTest, RunOp) {
     return std::string(R.Nodes.getNodeAs<Stmt>(Id) != nullptr ? "Bound"
                                                               : "Unbound");
   };
-  testExpr(Id, "3;", cat(run(SimpleFn)), "Bound");
+  testExpr(Id, "3;", run(SimpleFn), "Bound");
 }
 
 TEST(StencilToStringTest, RawTextOp) {
   auto S = cat("foo bar baz");
   StringRef Expected = R"("foo bar baz")";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, RawTextOpEscaping) {
   auto S = cat("foo \"bar\" baz\\n");
   StringRef Expected = R"("foo \"bar\" baz\\n")";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, DebugPrintNodeOp) {
-  auto S = cat(dPrint("Id"));
+  auto S = dPrint("Id");
   StringRef Expected = R"repr(dPrint("Id"))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, ExpressionOp) {
-  auto S = cat(expression("Id"));
+  auto S = expression("Id");
   StringRef Expected = R"repr(expression("Id"))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, DerefOp) {
-  auto S = cat(deref("Id"));
+  auto S = deref("Id");
   StringRef Expected = R"repr(deref("Id"))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, AddressOfOp) {
-  auto S = cat(addressOf("Id"));
+  auto S = addressOf("Id");
   StringRef Expected = R"repr(addressOf("Id"))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, SelectionOp) {
   auto S1 = cat(node("node1"));
-  EXPECT_EQ(S1.toString(), "selection(...)");
+  EXPECT_EQ(S1->toString(), "selection(...)");
 }
 
-TEST(StencilToStringTest, AccessOp) {
-  auto S = cat(access("Id", text("memberData")));
+TEST(StencilToStringTest, AccessOpText) {
+  auto S = access("Id", "memberData");
   StringRef Expected = R"repr(access("Id", "memberData"))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
-TEST(StencilToStringTest, AccessOpStencilPart) {
-  auto S = cat(access("Id", access("subId", "memberData")));
-  StringRef Expected = R"repr(access("Id", access("subId", "memberData")))repr";
-  EXPECT_EQ(S.toString(), Expected);
+TEST(StencilToStringTest, AccessOpSelector) {
+  auto S = access("Id", selection(name("otherId")));
+  StringRef Expected = R"repr(access("Id", selection(...)))repr";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
+TEST(StencilToStringTest, AccessOpStencil) {
+  auto S = access("Id", cat("foo_", "bar"));
+  StringRef Expected = R"repr(access("Id", seq("foo_", "bar")))repr";
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, IfBoundOp) {
-  auto S = cat(ifBound("Id", text("trueText"), access("exprId", "memberData")));
+  auto S = ifBound("Id", text("trueText"), access("exprId", "memberData"));
   StringRef Expected =
       R"repr(ifBound("Id", "trueText", access("exprId", "memberData")))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  EXPECT_EQ(S->toString(), Expected);
 }
 
 TEST(StencilToStringTest, RunOp) {
   auto F1 = [](const MatchResult &R) { return "foo"; };
-  auto S1 = cat(run(F1));
-  EXPECT_EQ(S1.toString(), "run(...)");
+  auto S1 = run(F1);
+  EXPECT_EQ(S1->toString(), "run(...)");
 }
 
-TEST(StencilToStringTest, MultipleOp) {
+TEST(StencilToStringTest, Sequence) {
   auto S = cat("foo", access("x", "m()"), "bar",
                ifBound("x", text("t"), access("e", "f")));
-  StringRef Expected = R"repr("foo", access("x", "m()"), "bar", )repr"
-                       R"repr(ifBound("x", "t", access("e", "f")))repr";
-  EXPECT_EQ(S.toString(), Expected);
+  StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
+                       R"repr(ifBound("x", "t", access("e", "f"))))repr";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
+TEST(StencilToStringTest, SequenceEmpty) {
+  auto S = cat();
+  StringRef Expected = "seq()";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
+TEST(StencilToStringTest, SequenceSingle) {
+  auto S = cat("foo");
+  StringRef Expected = "\"foo\"";
+  EXPECT_EQ(S->toString(), Expected);
+}
+
+TEST(StencilToStringTest, SequenceFromVector) {
+  auto S = catVector({text("foo"), access("x", "m()"), text("bar"),
+                      ifBound("x", text("t"), access("e", "f"))});
+  StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
+                       R"repr(ifBound("x", "t", access("e", "f"))))repr";
+  EXPECT_EQ(S->toString(), Expected);
 }
 } // namespace


        


More information about the cfe-commits mailing list