[libTooling] Add `access` and `ifBound` combinators to Stencil library.

[ymandel]

Summary

This revision add the access and ifBound combinators to the Stencil library:

• access -- constructs an idiomatic expression for accessing a member (a MemberExpr).
• ifBound -- chooses between two StencilParts based on the whether an id is bound in the match (corresponds to the combinator of the same name in RangeSelector).

[gribozavr]

clang/lib/Tooling/Refactoring/Stencil.cpp

70

I think for e = &e2 it will also produce e2.m... Maybe don't describe things in such detail.

"A stencil operation that builds a member access e.m or e->m as appropriate.

The expression is specified as a bound ID, the member is specified as a stencil."

I'm not even sure we need the second sentence.

198

These type aliases are only used once. Consider un-typealiasing.

[ymandel]

update to new signature for builders (Optional<string>)

[ymandel]

addressed comments.

[ymandel]

Thanks for the review!

clang/lib/Tooling/Refactoring/Stencil.cpp

198

Removed all of the aliases.

clang/include/clang/Tooling/Refactoring/Stencil.h

 StencilPart selection(RangeSelector Selector);
 
 /// \returns the source corresponding to the identified node.
 /// FIXME: Deprecated. Write `selection(node(Id))` instead.
 inline StencilPart node(llvm::StringRef Id) {
   return selection(tooling::node(Id));
 }
 
+/// 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) {
+  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);
+
+/// 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) {
+  return ifBound(Id, text(TrueText), text(FalseText));
+}
+
 /// For debug use only; semantics are not guaranteed.
 ///
 /// \returns the string resulting from calling the node's print() method.
 StencilPart dPrint(llvm::StringRef Id);
 } // namespace stencil
 } // namespace tooling
 } // namespace clang
 #endif // LLVM_CLANG_TOOLING_REFACTOR_STENCIL_H_

clang/lib/Tooling/Refactoring/Stencil.cpp

 //===--- Stencil.cpp - Stencil implementation -------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/Refactoring/Stencil.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Refactoring/SourceCode.h"
+#include "clang/Tooling/Refactoring/SourceCodeBuilders.h"
 #include "llvm/Support/Errc.h"
 #include <atomic>
 #include <memory>
 #include <string>
 
 using namespace clang;
 using namespace tooling;
 
 using ast_matchers::MatchFinder;
+using llvm::errc;
 using llvm::Error;
+using llvm::StringError;
 
 // A down_cast function to safely down cast a StencilPartInterface to a subclass
 // D. Returns nullptr if P is not an instance of D.
 template <typename D> const D *down_cast(const StencilPartInterface *P) {
   if (P == nullptr || D::typeId() != P->typeId())
     return nullptr;
   return static_cast<const D *>(P);
 }

 namespace {
 // An arbitrary fragment of code within a stencil.
 struct RawTextData {
   explicit RawTextData(std::string T) : Text(std::move(T)) {}
   std::string Text;
 };
 
 // A debugging operation to dump the AST for a particular (bound) AST node.
-struct DebugPrintNodeOpData {
-  explicit DebugPrintNodeOpData(std::string S) : Id(std::move(S)) {}
+struct DebugPrintNodeData {
+  explicit DebugPrintNodeData(std::string S) : Id(std::move(S)) {}
   std::string Id;
 };
 
 // The fragment of code corresponding to the selected range.
-struct SelectorOpData {
-  explicit SelectorOpData(RangeSelector S) : Selector(std::move(S)) {}
+struct SelectorData {
+  explicit SelectorData(RangeSelector S) : Selector(std::move(S)) {}
   RangeSelector Selector;
 };
+
+// A stencil operation that, given a reference to an expression e and a Part
+// describing a member m, yields "e->m", when e is a pointer, "e2->m" when e =
+// "*e2" and "e.m" otherwise.
+struct AccessData {
+  AccessData(StringRef BaseId, StencilPart Member)
+      : BaseId(BaseId), Member(std::move(Member)) {}
+  std::string BaseId;
+  StencilPart Member;
+};
+
+struct IfBoundData {
+  IfBoundData(StringRef Id, StencilPart TruePart, StencilPart FalsePart)
+      : Id(Id), TruePart(std::move(TruePart)), FalsePart(std::move(FalsePart)) {
+  }
+  std::string Id;
+  StencilPart TruePart;
+  StencilPart FalsePart;
+};
 } // namespace
 
 bool isEqualData(const RawTextData &A, const RawTextData &B) {
   return A.Text == B.Text;
 }
 
-bool isEqualData(const DebugPrintNodeOpData &A, const DebugPrintNodeOpData &B) {
+bool isEqualData(const DebugPrintNodeData &A, const DebugPrintNodeData &B) {
   return A.Id == B.Id;
 }
 
 // Equality is not (yet) defined for \c RangeSelector.
-bool isEqualData(const SelectorOpData &, const SelectorOpData &) { return false; }
+bool isEqualData(const SelectorData &, const SelectorData &) { return false; }
+
+bool isEqualData(const AccessData &A, const AccessData &B) {
+  return A.BaseId == B.BaseId && A.Member == B.Member;
+}
+
+bool isEqualData(const IfBoundData &A, const IfBoundData &B) {
+  return A.Id == B.Id && A.TruePart == B.TruePart && A.FalsePart == B.FalsePart;
+}
 
 // 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.
 
 Error evalData(const RawTextData &Data, const MatchFinder::MatchResult &,
                std::string *Result) {
   Result->append(Data.Text);
   return Error::success();
 }
 
-Error evalData(const DebugPrintNodeOpData &Data,
+Error evalData(const DebugPrintNodeData &Data,
                const MatchFinder::MatchResult &Match, std::string *Result) {
   std::string Output;
   llvm::raw_string_ostream Os(Output);
   auto NodeOrErr = getNode(Match.Nodes, Data.Id);
   if (auto Err = NodeOrErr.takeError())
     return Err;
   NodeOrErr->print(Os, PrintingPolicy(Match.Context->getLangOpts()));
   *Result += Os.str();
   return Error::success();
 }
 
-Error evalData(const SelectorOpData &Data, const MatchFinder::MatchResult &Match,
+Error evalData(const SelectorData &Data, const MatchFinder::MatchResult &Match,
                std::string *Result) {
   auto Range = Data.Selector(Match);
   if (!Range)
     return Range.takeError();
   *Result += getText(*Range, *Match.Context);
   return Error::success();
 }
 
+Error evalData(const AccessData &Data, const MatchFinder::MatchResult &Match,
+               std::string *Result) {
+  const auto *E = Match.Nodes.getNodeAs<Expr>(Data.BaseId);
+  if (E == nullptr)
+    return llvm::make_error<StringError>(errc::invalid_argument,
+                                         "Id not bound: " + Data.BaseId);
+  if (!E->isImplicitCXXThis()) {
+    if (llvm::Optional<std::string> S = E->getType()->isAnyPointerType()
+                                            ? buildArrow(*E, *Match.Context)
+                                            : buildDot(*E, *Match.Context))
+      Result += *S;
+    else
+      return llvm::make_error<StringError>(
+          errc::invalid_argument,
+          "Could not construct object text from ID: " + Data.BaseId);
+  }
+  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);
+}
+
 template <typename T>
 class StencilPartImpl : public StencilPartInterface {
   T Data;
 
 public:
   template <typename... Ps>
   explicit StencilPartImpl(Ps &&... Args)
       : StencilPartInterface(StencilPartImpl::typeId()),
bool isEqual(const StencilPartInterface &Other) const override {
     if (const auto *OtherPtr = down_cast<StencilPartImpl>(&Other))
       return isEqualData(Data, OtherPtr->Data);
     return false;
   }
 };
 
 namespace {
 using RawText = StencilPartImpl<RawTextData>;
-using DebugPrintNodeOp = StencilPartImpl<DebugPrintNodeOpData>;
-using SelectorOp = StencilPartImpl<SelectorOpData>;
+using DebugPrintNodeOp = StencilPartImpl<DebugPrintNodeData>;
+using SelectorOp = StencilPartImpl<SelectorData>;
+using AccessOp = StencilPartImpl<AccessData>;
+using IfBoundOp = StencilPartImpl<IfBoundData>;
 } // namespace
 
 StencilPart Stencil::wrap(StringRef Text) {
   return stencil::text(Text);
 }
 
 StencilPart Stencil::wrap(RangeSelector Selector) {
   return stencil::selection(std::move(Selector));

 
 StencilPart stencil::selection(RangeSelector Selector) {
   return StencilPart(std::make_shared<SelectorOp>(std::move(Selector)));
 }
 
 StencilPart stencil::dPrint(StringRef Id) {
   return StencilPart(std::make_shared<DebugPrintNodeOp>(Id));
 }
+
+StencilPart stencil::access(StringRef BaseId, StencilPart Member) {
+  return StencilPart(std::make_shared<AccessOp>(BaseId, std::move(Member)));
+}
+
+StencilPart stencil::ifBound(StringRef Id, StencilPart TruePart,
+                             StencilPart FalsePart) {
+  return StencilPart(std::make_shared<IfBoundOp>(Id, std::move(TruePart),
+                                                 std::move(FalsePart)));
+}

clang/unittests/Tooling/StencilTest.cpp

 //===- unittest/Tooling/StencilTest.cpp -----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/Refactoring/Stencil.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace tooling;
 using namespace ast_matchers;
 
 namespace {
+using ::llvm::HasValue;
 using ::testing::AllOf;
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using MatchResult = MatchFinder::MatchResult;
+using stencil::access;
 using stencil::cat;
 using stencil::dPrint;
+using stencil::ifBound;
 using stencil::text;
 
 // In tests, we can't directly match on llvm::Expected since its accessors
 // mutate the object. So, we collapse it to an Optional.
 static llvm::Optional<std::string> toOptional(llvm::Expected<std::string> V) {
   if (V)
     return *V;
   ADD_FAILURE() << "Losing error in conversion to IsSomething: "
                 << llvm::toString(V.takeError());
   return llvm::None;
 }
 
 // A very simple matcher for llvm::Optional values.
 MATCHER_P(IsSomething, ValueMatcher, "") {
   if (!arg)
     return false;
   return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
 }
 
 // Create a valid translation-unit from a statement.
-static std::string wrapSnippet(llvm::Twine StatementCode) {
-  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+static std::string wrapSnippet(StringRef StatementCode) {
+  return ("struct S { int field; }; auto stencil_test_snippet = []{" +
+          StatementCode + "};")
+      .str();
 }
 
 static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
   return varDecl(hasName("stencil_test_snippet"),
                  hasDescendant(compoundStmt(hasAnySubstatement(Matcher))));
 }
 
 struct TestMatch {
   // The AST unit from which `result` is built. We bundle it because it backs
   // the result. Users are not expected to access it.
   std::unique_ptr<ASTUnit> AstUnit;
   // The result to use in the test. References `ast_unit`.
   MatchResult Result;
 };
 
 // Matches `Matcher` against the statement `StatementCode` and returns the
 // result. Handles putting the statement inside a function and modifying the
-// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
-// that is, produce exactly one match.
-static llvm::Optional<TestMatch> matchStmt(llvm::Twine StatementCode,
+// matcher correspondingly. `Matcher` should match one of the statements in
+// `StatementCode` exactly -- that is, produce exactly one match. However,
+// `StatementCode` may contain other statements not described by `Matcher`.
+static llvm::Optional<TestMatch> matchStmt(StringRef StatementCode,
                                            StatementMatcher Matcher) {
   auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
   if (AstUnit == nullptr) {
     ADD_FAILURE() << "AST construction failed";
     return llvm::None;
   }
   ASTContext &Context = AstUnit->getASTContext();
   auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
       if (Err) {
         ADD_FAILURE() << "Unhandled error: " << llvm::toString(std::move(Err));
       }
     }
   }
 
   // Tests failures caused by references to unbound nodes. `unbound_id` is the
   // id that will cause the failure.
-  void testUnboundNodeError(const Stencil &Stencil, llvm::StringRef UnboundId) {
+  void testUnboundNodeError(const Stencil &Stencil, StringRef UnboundId) {
     testError(Stencil, AllOf(HasSubstr(UnboundId), HasSubstr("not bound")));
   }
 };
 
 TEST_F(StencilTest, SingleStatement) {
   StringRef Condition("C"), Then("T"), Else("E");
   const std::string Snippet = R"cc(
     if (true)

 }
 
 // Tests that a stencil with a single parameter (`Id`) evaluates to the expected
 // string, when `Id` is bound to the expression-statement in `Snippet`.
 void testExpr(StringRef Id, StringRef Snippet, const Stencil &Stencil,
               StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
   ASSERT_TRUE(StmtMatch);
-  EXPECT_THAT(toOptional(Stencil.eval(StmtMatch->Result)),
-              IsSomething(Expected));
+  EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
 }
 
 TEST_F(StencilTest, SelectionOp) {
   StringRef Id = "id";
   testExpr(Id, "3;", cat(node(Id)), "3");
 }
 
+TEST_F(StencilTest, IfBoundOpBound) {
+  StringRef Id = "id";
+  testExpr(Id, "3;", cat(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");
+}
+
+TEST_F(StencilTest, AccessOpValue) {
+  StringRef Snippet = R"cc(
+    S x;
+    x;
+  )cc";
+  StringRef Id = "id";
+  testExpr(Id, Snippet, cat(access(Id, "field")), "x.field");
+}
+
+TEST_F(StencilTest, AccessOpValueExplicitText) {
+  StringRef Snippet = R"cc(
+    S x;
+    x;
+  )cc";
+  StringRef Id = "id";
+  testExpr(Id, Snippet, cat(access(Id, text("field"))), "x.field");
+}
+
+TEST_F(StencilTest, AccessOpValueAddress) {
+  StringRef Snippet = R"cc(
+    S x;
+    &x;
+  )cc";
+  StringRef Id = "id";
+  testExpr(Id, Snippet, cat(access(Id, "field")), "x.field");
+}
+
+TEST_F(StencilTest, AccessOpPointer) {
+  StringRef Snippet = R"cc(
+    S *x;
+    x;
+  )cc";
+  StringRef Id = "id";
+  testExpr(Id, Snippet, cat(access(Id, "field")), "x->field");
+}
+
+TEST_F(StencilTest, AccessOpPointerDereference) {
+  StringRef Snippet = R"cc(
+    S *x;
+    *x;
+  )cc";
+  StringRef Id = "id";
+  testExpr(Id, Snippet, cat(access(Id, "field")), "x->field");
+}
+
+TEST_F(StencilTest, AccessOpExplicitThis) {
+  using clang::ast_matchers::hasObjectExpression;
+  using clang::ast_matchers::memberExpr;
+
+  // Set up the code so we can bind to a use of this.
+  StringRef Snippet = R"cc(
+    class C {
+     public:
+      int x;
+      int foo() { return this->x; }
+    };
+  )cc";
+  auto StmtMatch =
+      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("this->field"));
+}
+
+TEST_F(StencilTest, AccessOpImplicitThis) {
+  using clang::ast_matchers::hasObjectExpression;
+  using clang::ast_matchers::memberExpr;
+
+  // Set up the code so we can bind to a use of (implicit) this.
+  StringRef Snippet = R"cc(
+    class C {
+     public:
+      int x;
+      int foo() { return x; }
+    };
+  )cc";
+  auto StmtMatch =
+      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"));
+}
+
 TEST(StencilEqualityTest, Equality) {
   auto Lhs = cat("foo", dPrint("dprint_id"));
   auto Rhs = cat("foo", dPrint("dprint_id"));
   EXPECT_EQ(Lhs, Rhs);
 }
 
 TEST(StencilEqualityTest, InEqualityDifferentOrdering) {
   auto Lhs = cat("foo", dPrint("node"));