[libTooling] Adds more support for constructing object access expressions.

[ymandel]

Summary

This patch adds a buildAccess function, which constructs a string with the
proper operator to use based on the expression's form and type. It also adds two
predicates related to smart pointers, which are needed by buildAccess but are
also of general value.

[ymandel]

ignore implicit and add deprecations

[asoffer]

clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp

73

Naming nit: This could be confusing for anyone not familiar with the "duck typing" idiom.
BehavesLikeASmartPointer?

78

I think std::optional satisfies these requirements but should not be considered a smart-pointer.

79

The known smart pointers quack like smart pointers, so this is redundant, right?

[gribozavr2]

clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp

78

Are you objecting to naming, or to this code handling optionals and smart pointers in the same way at all?

An optional is not a pointer, however it has few semantic differences from std::unique_ptr that are relevant here (whether the object is heap-allocated or not is not relevant for building an access expression). So I think this code should handle optionals and other value wrappers like llvm::Expected and absl::StatusOr.

199–200

212

clang/unittests/Tooling/SourceCodeBuildersTest.cpp

148

There are actually two expressions in the snippet above, one is the DeclRefExpr in P;, but the other is the CXXConstructExpr in std::unique_ptr<int> P;. Both have the same type, so it makes no difference which one we find, but the snippet deliberately having P; makes it look like we specifically want the DeclRefExpr. I'd suggest changing the matcher to declRefExpr() for clarity. Same in tests below.

360

This is a case where the old APIs provided the user with a choice, but the new API does not. If the user wanted to call a method on the smart pointer itself (e.g., reset()), they could have used buildDot to get x..

IDK if it is an important use case, but I thought I'd mention it, since the new API is not 100% equivalent to the ones that it replaces.

[ymandel]

clang/unittests/Tooling/SourceCodeBuildersTest.cpp

360

Agreed. I think it is important and I should add a (defaulted) parameter to buildAccess that forces "smart" pointers to be treated like any other value. WDYT?

[gribozavr2]

clang/unittests/Tooling/SourceCodeBuildersTest.cpp

360

Yes, something like AllowDereferencingSmartPointers.

[ymandel]

addressed comments.

[ymandel]

clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp

78

I went with a strict list of known names, but expanded the list as specified in the comments. Also, I explicitly note that API doesn't guarantee stability of that list (over time).

clang/unittests/Tooling/SourceCodeBuildersTest.cpp

71

drive-by fix to silence distracting warnings in the tests.

360

went with PLTClass for an enum name, but happy to take alternative suggestions (see the header file).

[ymandel]

extended list of pointer-like types

[ymandel]

Fix StencilTests to align with new semantics

[ymandel]

fix bad previous snapshot

clang/include/clang/Tooling/Transformer/SourceCodeBuilders.h

 /// asking whether it ever needs parens.
 inline bool needParensBeforeDotOrArrow(const Expr &E) {
   return mayEverNeedParens(E);
 }
 
 /// Determines whether printing this expression to the right of a unary operator
 /// requires a parentheses to preserve its meaning.
 bool needParensAfterUnaryOperator(const Expr &E);
+
+// Recognizes known types (and sugared versions thereof) that overload the `*`
+// and `->` operator. Currently, included types are exactly:
+//
+// * std::unique_ptr, std::shared_ptr, std::weak_ptr,
+// * std::optional, absl::optional.
+bool isKnownPointerLikeType(QualType Ty, ASTContext &Context);
 /// @}
 
 /// \name Basic code-string generation utilities.
 /// @{
 
 /// Builds source for an expression, adding parens if needed for unambiguous
 /// parsing.
 llvm::Optional<std::string> buildParens(const Expr &E,
llvm::Optional<std::string> buildAddressOf(const Expr &E,
                                            const ASTContext &Context);
 
 /// Adds a dot to the end of the given expression, but adds parentheses when
 /// needed by the syntax, and simplifies to `->` when possible, e.g.:
 ///
 ///  `x` becomes `x.`
 ///  `*a` becomes `a->`
 ///  `a+b` becomes `(a+b).`
+///
+/// DEPRECATED. Use `buildAccess`.
 llvm::Optional<std::string> buildDot(const Expr &E, const ASTContext &Context);
 
 /// Adds an arrow to the end of the given expression, but adds parentheses
 /// when needed by the syntax, and simplifies to `.` when possible, e.g.:
 ///
 ///  `x` becomes `x->`
 ///  `&a` becomes `a.`
 ///  `a+b` becomes `(a+b)->`
+///
+/// DEPRECATED. Use `buildAccess`.
 llvm::Optional<std::string> buildArrow(const Expr &E,
                                        const ASTContext &Context);
+
+/// Specifies how to classify pointer-like types -- like values or like pointers
+/// -- with regard to generating member-access syntax.
+enum class PLTClass : bool {
+  Value,
+  Pointer,
+};
+
+/// Adds an appropriate access operator (`.`, `->` or nothing, in the case of
+/// implicit `this`) to the end of the given expression. Adds parentheses when
+/// needed by the syntax and simplifies when possible. If `PLTypeClass` is
+/// `Pointer`, for known pointer-like types (see `isKnownPointerLikeType`),
+/// treats `operator->` and `operator*` like the built-in `->` and `*`
+/// operators.
+///
+///  `x` becomes `x->` or `x.`, depending on `E`'s type
+///  `a+b` becomes `(a+b)->` or `(a+b).`, depending on `E`'s type
+///  `&a` becomes `a.`
+///  `*a` becomes `a->`
+llvm::Optional<std::string>
+buildAccess(const Expr &E, ASTContext &Context,
+            PLTClass Classification = PLTClass::Pointer);
 /// @}
 
 } // namespace tooling
 } // namespace clang
 #endif // LLVM_CLANG_TOOLING_TRANSFORMER_SOURCECODEBUILDERS_H

clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp

 //===--- SourceCodeBuilder.cpp ----------------------------------*- 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/Transformer/SourceCodeBuilders.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/SourceCode.h"
 #include "llvm/ADT/Twine.h"
 #include <string>
 
 using namespace clang;
 using namespace tooling;
 
 const Expr *tooling::reallyIgnoreImplicit(const Expr &E) {
bool tooling::needParensAfterUnaryOperator(const Expr &E) {
   if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(Expr))
     return Op->getNumArgs() == 2 && Op->getOperator() != OO_PlusPlus &&
            Op->getOperator() != OO_MinusMinus && Op->getOperator() != OO_Call &&
            Op->getOperator() != OO_Subscript;
 
   return false;
 }
 
+bool tooling::isKnownPointerLikeType(QualType Ty, ASTContext &Context) {
+  using namespace ast_matchers;
+  const auto PointerLikeTy =
+      type(hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+          hasAnyName("::std::unique_ptr", "::std::shared_ptr",
+                     "::std::weak_ptr", "std::optional", "absl::optional"))))));
+  return match(PointerLikeTy, Ty, Context).size() > 0;
+}
+
 llvm::Optional<std::string> tooling::buildParens(const Expr &E,
                                                  const ASTContext &Context) {
   StringRef Text = getText(E, Context);
   if (Text.empty())
     return llvm::None;
   if (mayEverNeedParens(E))
     return ("(" + Text + ")").str();
   return Text.str();
 }
 
 llvm::Optional<std::string>
 tooling::buildDereference(const Expr &E, const ASTContext &Context) {
   if (const auto *Op = dyn_cast<UnaryOperator>(&E))
     if (Op->getOpcode() == UO_AddrOf) {
llvm::Optional<std::string> tooling::buildAddressOf(const Expr &E,
   if (Text.empty())
     return llvm::None;
   if (needParensAfterUnaryOperator(E)) {
     return ("&(" + Text + ")").str();
   }
   return ("&" + Text).str();
 }
 
-llvm::Optional<std::string> tooling::buildDot(const Expr &E,
-                                              const ASTContext &Context) {
+// Append the appropriate access operation (syntactically) to `E`, assuming `E`
+// is a non-pointer value.
+static llvm::Optional<std::string>
+buildAccessForValue(const Expr &E, const ASTContext &Context) {
   if (const auto *Op = llvm::dyn_cast<UnaryOperator>(&E))
     if (Op->getOpcode() == UO_Deref) {
       // Strip leading '*', add following '->'.
       const Expr *SubExpr = Op->getSubExpr()->IgnoreParenImpCasts();
       StringRef DerefText = getText(*SubExpr, Context);
       if (DerefText.empty())
         return llvm::None;
       if (needParensBeforeDotOrArrow(*SubExpr))
         return ("(" + DerefText + ")->").str();
       return (DerefText + "->").str();
     }
 
   // Add following '.'.
   StringRef Text = getText(E, Context);
   if (Text.empty())
     return llvm::None;
   if (needParensBeforeDotOrArrow(E)) {
     return ("(" + Text + ").").str();
   }
   return (Text + ".").str();
 }
 
-llvm::Optional<std::string> tooling::buildArrow(const Expr &E,
-                                                const ASTContext &Context) {
+// Append the appropriate access operation (syntactically) to `E`, assuming `E`
+// is a pointer value.
+static llvm::Optional<std::string>
+buildAccessForPointer(const Expr &E, const ASTContext &Context) {
   if (const auto *Op = llvm::dyn_cast<UnaryOperator>(&E))
     if (Op->getOpcode() == UO_AddrOf) {
       // Strip leading '&', add following '.'.
       const Expr *SubExpr = Op->getSubExpr()->IgnoreParenImpCasts();
       StringRef DerefText = getText(*SubExpr, Context);
       if (DerefText.empty())
         return llvm::None;
       if (needParensBeforeDotOrArrow(*SubExpr))
         return ("(" + DerefText + ").").str();
       return (DerefText + ".").str();
     }
 
   // Add following '->'.
   StringRef Text = getText(E, Context);
   if (Text.empty())
     return llvm::None;
   if (needParensBeforeDotOrArrow(E))
     return ("(" + Text + ")->").str();
   return (Text + "->").str();
 }
+
+llvm::Optional<std::string> tooling::buildDot(const Expr &E,
+                                              const ASTContext &Context) {
+  return buildAccessForValue(E, Context);
+}
+
+llvm::Optional<std::string> tooling::buildArrow(const Expr &E,
+                                                const ASTContext &Context) {
+  return buildAccessForPointer(E, Context);
+}
+
+// If `E` is an overloaded-operator call of kind `K` on an object `O`, returns
+// `O`. Otherwise, returns `nullptr`.
+static const Expr *maybeGetOperatorObjectArg(const Expr &E,
+                                             OverloadedOperatorKind K) {
+  if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(&E)) {
+    if (OpCall->getOperator() == K && OpCall->getNumArgs() == 1)
+      return OpCall->getArg(0);
+  }
+  return nullptr;
+}
+
+static bool treatLikePointer(QualType Ty, PLTClass C, ASTContext &Context) {
+  switch (C) {
+  case PLTClass::Value:
+    return false;
+  case PLTClass::Pointer:
+    return isKnownPointerLikeType(Ty, Context);
+  }
+}
+
+// FIXME: move over the other `maybe` functionality from Stencil. Should all be
+// in one place.
+llvm::Optional<std::string> tooling::buildAccess(const Expr &RawExpression,
+                                                 ASTContext &Context,
+                                                 PLTClass Classification) {
+  if (RawExpression.isImplicitCXXThis())
+    // Return the empty string, because `None` signifies some sort of failure.
+    return std::string();
+
+  const Expr *E = RawExpression.IgnoreImplicitAsWritten();
+
+  if (E->getType()->isAnyPointerType() ||
+      treatLikePointer(E->getType(), Classification, Context)) {
+    // Strip off operator-> calls. They can only occur inside an actual arrow
+    // member access, so we treat them as equivalent to an actual object
+    // expression.
+    if (const auto *Obj = maybeGetOperatorObjectArg(*E, clang::OO_Arrow))
+      E = Obj;
+    return buildAccessForPointer(*E, Context);
+  }
+
+  if (const auto *Obj = maybeGetOperatorObjectArg(*E, clang::OO_Star)) {
+    if (treatLikePointer(Obj->getType(), Classification, Context))
+      return buildAccessForPointer(*Obj, Context);
+  };
+
+  return buildAccessForValue(*E, Context);
+}

clang/lib/Tooling/Transformer/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/Transformer/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/Basic/SourceLocation.h"
 #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 "llvm/Support/Error.h"
static Error printNode(StringRef Id, const MatchFinder::MatchResult &Match,
   auto NodeOrErr = getNode(Match.Nodes, Id);
   if (auto Err = NodeOrErr.takeError())
     return Err;
   NodeOrErr->print(Os, PrintingPolicy(Match.Context->getLangOpts()));
   *Result += Os.str();
   return Error::success();
 }
 
-// FIXME: Consider memoizing this function using the `ASTContext`.
-static bool isSmartPointerType(QualType Ty, ASTContext &Context) {
-  using namespace ::clang::ast_matchers;
-
-  // Optimization: hard-code common smart-pointer types. This can/should be
-  // removed if we start caching the results of this function.
-  auto KnownSmartPointer =
-      cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
-  const auto QuacksLikeASmartPointer = cxxRecordDecl(
-      hasMethod(cxxMethodDecl(hasOverloadedOperatorName("->"),
-                              returns(qualType(pointsTo(type()))))),
-      hasMethod(cxxMethodDecl(hasOverloadedOperatorName("*"),
-                              returns(qualType(references(type()))))));
-  const auto SmartPointer = qualType(hasDeclaration(
-      cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer))));
-  return match(SmartPointer, Ty, Context).size() > 0;
-}
-
-// Identifies use of `operator*` on smart pointers, and returns the underlying
-// smart-pointer expression; otherwise, returns null.
-static const Expr *isSmartDereference(const Expr &E, ASTContext &Context) {
-  using namespace ::clang::ast_matchers;
-
-  const auto HasOverloadedArrow = cxxRecordDecl(hasMethod(cxxMethodDecl(
-      hasOverloadedOperatorName("->"), returns(qualType(pointsTo(type()))))));
-  // Verify it is a smart pointer by finding `operator->` in the class
-  // declaration.
-  auto Deref = cxxOperatorCallExpr(
-      hasOverloadedOperatorName("*"), hasUnaryOperand(expr().bind("arg")),
-      callee(cxxMethodDecl(ofClass(HasOverloadedArrow))));
-  return selectFirst<Expr>("arg", match(Deref, E, Context));
-}
-
 namespace {
 // An arbitrary fragment of code within a stencil.
 class RawTextStencil : public StencilInterface {
   std::string Text;
 
 public:
   explicit RawTextStencil(std::string T) : Text(std::move(T)) {}
 
Error eval(const MatchFinder::MatchResult &Match,
     case UnaryNodeOperator::Parens:
       Source = tooling::buildParens(*E, *Match.Context);
       break;
     case UnaryNodeOperator::Deref:
       Source = tooling::buildDereference(*E, *Match.Context);
       break;
     case UnaryNodeOperator::MaybeDeref:
       if (E->getType()->isAnyPointerType() ||
-          isSmartPointerType(E->getType(), *Match.Context)) {
+          tooling::isKnownPointerLikeType(E->getType(), *Match.Context)) {
         // Strip off any operator->. This can only occur inside an actual arrow
         // member access, so we treat it as equivalent to an actual object
         // expression.
         if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(E)) {
           if (OpCall->getOperator() == clang::OO_Arrow &&
               OpCall->getNumArgs() == 1) {
             E = OpCall->getArg(0);
           }
         }
         Source = tooling::buildDereference(*E, *Match.Context);
         break;
       }
       *Result += tooling::getText(*E, *Match.Context);
       return Error::success();
     case UnaryNodeOperator::AddressOf:
       Source = tooling::buildAddressOf(*E, *Match.Context);
       break;
     case UnaryNodeOperator::MaybeAddressOf:
       if (E->getType()->isAnyPointerType() ||
-          isSmartPointerType(E->getType(), *Match.Context)) {
+          tooling::isKnownPointerLikeType(E->getType(), *Match.Context)) {
         // Strip off any operator->. This can only occur inside an actual arrow
         // member access, so we treat it as equivalent to an actual object
         // expression.
         if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(E)) {
           if (OpCall->getOperator() == clang::OO_Arrow &&
               OpCall->getNumArgs() == 1) {
             E = OpCall->getArg(0);
           }
public:
   }
 
   Error eval(const MatchFinder::MatchResult &Match,
              std::string *Result) const override {
     const auto *E = Match.Nodes.getNodeAs<Expr>(BaseId);
     if (E == nullptr)
       return llvm::make_error<StringError>(errc::invalid_argument,
                                            "Id not bound: " + BaseId);
-    if (!E->isImplicitCXXThis()) {
-      llvm::Optional<std::string> S;
-      if (E->getType()->isAnyPointerType() ||
-          isSmartPointerType(E->getType(), *Match.Context)) {
-        // Strip off any operator->. This can only occur inside an actual arrow
-        // member access, so we treat it as equivalent to an actual object
-        // expression.
-        if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(E)) {
-          if (OpCall->getOperator() == clang::OO_Arrow &&
-              OpCall->getNumArgs() == 1) {
-            E = OpCall->getArg(0);
-          }
-        }
-        S = tooling::buildArrow(*E, *Match.Context);
-      } else if (const auto *Operand = isSmartDereference(*E, *Match.Context)) {
-        // `buildDot` already handles the built-in dereference operator, so we
-        // only need to catch overloaded `operator*`.
-        S = tooling::buildArrow(*Operand, *Match.Context);
-      } else {
-        S = tooling::buildDot(*E, *Match.Context);
-      }
-      if (S.hasValue())
-        *Result += *S;
-      else
-        return llvm::make_error<StringError>(
-            errc::invalid_argument,
-            "Could not construct object text from ID: " + BaseId);
-    }
+    llvm::Optional<std::string> S = tooling::buildAccess(*E, *Match.Context);
+    if (!S.hasValue())
+    *Result += *S;
     return Member->eval(Match, Result);
   }
 };
 
 class IfBoundStencil : public StencilInterface {
   std::string Id;
   Stencil TrueStencil;
   Stencil FalseStencil;

clang/unittests/Tooling/SourceCodeBuildersTest.cpp

 //===- unittest/Tooling/SourceCodeBuildersTest.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/Transformer/SourceCodeBuilders.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace tooling;
 using namespace ast_matchers;
 
 namespace {
 using MatchResult = MatchFinder::MatchResult;
 using llvm::ValueIs;
 
 // Create a valid translation unit from a statement.
 static std::string wrapSnippet(StringRef StatementCode) {
-  return ("struct S { S(); S(int); int field; };\n"
+  return ("namespace std {\n"
+          "template <typename T> struct unique_ptr {\n"
+          "  T* operator->() const;\n"
+          "  T& operator*() const;\n"
+          "};\n"
+          "template <typename T> struct shared_ptr {\n"
+          "  T* operator->() const;\n"
+          "  T& operator*() const;\n"
+          "};\n"
+          "}\n"
+          "struct A { void super(); };\n"
+          "struct S : public A { S(); S(int); int Field; };\n"
           "S operator+(const S &a, const S &b);\n"
+          "struct Smart {\n"
+          "  S* operator->() const;\n"
+          "  S& operator*() const;\n"
+          "};\n"
           "auto test_snippet = []{" +
           StatementCode + "};")
       .str();
 }
 
 static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
   return varDecl(hasName("test_snippet"),
                  hasDescendant(compoundStmt(hasAnySubstatement(Matcher))));

 
 // 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 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));
+  auto AstUnit = buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
+                                          {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
     ADD_FAILURE() << "AST construction failed";
     return llvm::None;
   }
   ASTContext &Context = AstUnit->getASTContext();
   auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
   // We expect a single, exact match for the statement.
   if (Matches.size() != 1) {
TEST(SourceCodeBuildersTest, needParensAfterUnaryOperator) {
   testPredicate(needParensAfterUnaryOperator, "3 + 5;", true);
   testPredicate(needParensAfterUnaryOperator, "true ? 3 : 5;", true);
   testPredicate(needParensAfterUnaryOperator, "S(3) + S(5);", true);
 
   testPredicate(needParensAfterUnaryOperator, "int x; x;", false);
   testPredicate(needParensAfterUnaryOperator, "int(3.0);", false);
   testPredicate(needParensAfterUnaryOperator, "void f(); f();", false);
   testPredicate(needParensAfterUnaryOperator, "int a[3]; a[0];", false);
-  testPredicate(needParensAfterUnaryOperator, "S x; x.field;", false);
+  testPredicate(needParensAfterUnaryOperator, "S x; x.Field;", false);
   testPredicate(needParensAfterUnaryOperator, "int x = 1; --x;", false);
   testPredicate(needParensAfterUnaryOperator, "int x = 1; -x;", false);
 }
 
 TEST(SourceCodeBuildersTest, needParensAfterUnaryOperatorInImplicitConversion) {
   // The binary operation will be embedded in various implicit
   // expressions. Verify they are ignored.
   testPredicateOnArg(needParensAfterUnaryOperator, "void f(S); f(3 + 5);",
                      true);
 }
 
 TEST(SourceCodeBuildersTest, mayEverNeedParens) {
   testPredicate(mayEverNeedParens, "3 + 5;", true);
   testPredicate(mayEverNeedParens, "true ? 3 : 5;", true);
   testPredicate(mayEverNeedParens, "int x = 1; --x;", true);
   testPredicate(mayEverNeedParens, "int x = 1; -x;", true);
 
   testPredicate(mayEverNeedParens, "int x; x;", false);
   testPredicate(mayEverNeedParens, "int(3.0);", false);
   testPredicate(mayEverNeedParens, "void f(); f();", false);
   testPredicate(mayEverNeedParens, "int a[3]; a[0];", false);
-  testPredicate(mayEverNeedParens, "S x; x.field;", false);
+  testPredicate(mayEverNeedParens, "S x; x.Field;", false);
 }
 
 TEST(SourceCodeBuildersTest, mayEverNeedParensInImplictConversion) {
   // The binary operation will be embedded in various implicit
   // expressions. Verify they are ignored.
   testPredicateOnArg(mayEverNeedParens, "void f(S); f(3 + 5);", true);
 }
 
+TEST(SourceCodeBuildersTest, isKnownPointerLikeTypeUniquePtr) {
+  std::string Snippet = "std::unique_ptr<int> P; P;";
+  auto StmtMatch =
+      matchStmt(Snippet, declRefExpr(hasType(qualType().bind("ty"))));
+  ASSERT_TRUE(StmtMatch) << "Snippet: " << Snippet;
+  EXPECT_TRUE(
+      isKnownPointerLikeType(*StmtMatch->Result.Nodes.getNodeAs<QualType>("ty"),
+                             *StmtMatch->Result.Context))
+      << "Snippet: " << Snippet;
+}
+
+TEST(SourceCodeBuildersTest, isKnownPointerLikeTypeSharedPtr) {
+  std::string Snippet = "std::shared_ptr<int> P; P;";
+  auto StmtMatch =
+      matchStmt(Snippet, declRefExpr(hasType(qualType().bind("ty"))));
+  ASSERT_TRUE(StmtMatch) << "Snippet: " << Snippet;
+  EXPECT_TRUE(
+      isKnownPointerLikeType(*StmtMatch->Result.Nodes.getNodeAs<QualType>("ty"),
+                             *StmtMatch->Result.Context))
+      << "Snippet: " << Snippet;
+}
+
+TEST(SourceCodeBuildersTest, isKnownPointerLikeTypeUnknownTypeFalse) {
+  std::string Snippet = "Smart P; P;";
+  auto StmtMatch =
+      matchStmt(Snippet, declRefExpr(hasType(qualType().bind("ty"))));
+  ASSERT_TRUE(StmtMatch) << "Snippet: " << Snippet;
+  EXPECT_FALSE(
+      isKnownPointerLikeType(*StmtMatch->Result.Nodes.getNodeAs<QualType>("ty"),
+                             *StmtMatch->Result.Context))
+      << "Snippet: " << Snippet;
+}
+
+TEST(SourceCodeBuildersTest, isKnownPointerLikeTypeNormalTypeFalse) {
+  std::string Snippet = "int *P; P;";
+  auto StmtMatch =
+      matchStmt(Snippet, declRefExpr(hasType(qualType().bind("ty"))));
+  ASSERT_TRUE(StmtMatch) << "Snippet: " << Snippet;
+  EXPECT_FALSE(
+      isKnownPointerLikeType(*StmtMatch->Result.Nodes.getNodeAs<QualType>("ty"),
+                             *StmtMatch->Result.Context))
+      << "Snippet: " << Snippet;
+}
+
 static void testBuilder(
     llvm::Optional<std::string> (*Builder)(const Expr &, const ASTContext &),
     StringRef Snippet, StringRef Expected) {
   auto StmtMatch = matchStmt(Snippet, expr().bind("expr"));
   ASSERT_TRUE(StmtMatch);
   EXPECT_THAT(Builder(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
                       *StmtMatch->Result.Context),
               ValueIs(std::string(Expected)));
 }
 
+static void testBuildAccess(StringRef Snippet, StringRef Expected,
+                            PLTClass C = PLTClass::Pointer) {
+  auto StmtMatch = matchStmt(Snippet, expr().bind("expr"));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT(buildAccess(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
+                          *StmtMatch->Result.Context, C),
+              ValueIs(std::string(Expected)));
+}
+
 TEST(SourceCodeBuildersTest, BuildParensUnaryOp) {
   testBuilder(buildParens, "-4;", "(-4)");
 }
 
 TEST(SourceCodeBuildersTest, BuildParensBinOp) {
   testBuilder(buildParens, "4 + 4;", "(4 + 4)");
 }
 

 
 TEST(SourceCodeBuildersTest, BuildArrowBinaryOperation) {
   testBuilder(buildArrow, "S *x; x + 1;", "(x + 1)->");
 }
 
 TEST(SourceCodeBuildersTest, BuildArrowValueAddressWithParens) {
   testBuilder(buildArrow, "S x; &(true ? x : x);", "(true ? x : x).");
 }
+
+TEST(SourceCodeBuildersTest, BuildAccessValue) {
+  testBuildAccess("S x; x;", "x.");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessPointerDereference) {
+  testBuildAccess("S *x; *x;", "x->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessPointerDereferenceIgnoresParens) {
+  testBuildAccess("S *x; *(x);", "x->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessValueBinaryOperation) {
+  testBuildAccess("S x; x + x;", "(x + x).");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessPointerDereferenceExprWithParens) {
+  testBuildAccess("S *x; *(x + 1);", "(x + 1)->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessPointer) {
+  testBuildAccess("S *x; x;", "x->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessValueAddress) {
+  testBuildAccess("S x; &x;", "x.");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessValueAddressIgnoresParens) {
+  testBuildAccess("S x; &(x);", "x.");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessPointerBinaryOperation) {
+  testBuildAccess("S *x; x + 1;", "(x + 1)->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessValueAddressWithParens) {
+  testBuildAccess("S x; &(true ? x : x);", "(true ? x : x).");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointer) {
+  testBuildAccess("std::unique_ptr<int> x; x;", "x->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointerAsValue) {
+  testBuildAccess("std::unique_ptr<int> x; x;", "x.", PLTClass::Value);
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointerDeref) {
+  testBuildAccess("std::unique_ptr<int> x; *x;", "x->");
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointerDerefAsValue) {
+  testBuildAccess("std::unique_ptr<int> x; *x;", "(*x).", PLTClass::Value);
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessSmartPointerMemberCall) {
+  StringRef Snippet = R"cc(
+    Smart x;
+    x->Field;
+  )cc";
+  auto StmtMatch =
+      matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind("expr"))));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT(buildAccess(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
+                          *StmtMatch->Result.Context),
+              ValueIs(std::string("x->")));
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessIgnoreImplicit) {
+  StringRef Snippet = R"cc(
+    S x;
+    A *a;
+    a = &x;
+  )cc";
+  auto StmtMatch =
+      matchStmt(Snippet, binaryOperator(isAssignmentOperator(),
+                                        hasRHS(expr().bind("expr"))));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT(buildAccess(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
+                          *StmtMatch->Result.Context),
+              ValueIs(std::string("x.")));
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessImplicitThis) {
+  StringRef Snippet = R"cc(
+    struct Struct {
+      void foo() {}
+      void bar() {
+        foo();
+      }
+    };
+  )cc";
+  auto StmtMatch = matchStmt(
+      Snippet,
+      cxxMemberCallExpr(onImplicitObjectArgument(cxxThisExpr().bind("expr"))));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT(buildAccess(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
+                          *StmtMatch->Result.Context),
+              ValueIs(std::string()));
+}
+
+TEST(SourceCodeBuildersTest, BuildAccessImplicitThisIgnoreImplicitCasts) {
+  StringRef Snippet = "struct B : public A { void f() { super(); } };";
+  auto StmtMatch = matchStmt(
+      Snippet,
+      cxxMemberCallExpr(onImplicitObjectArgument(expr().bind("expr"))));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT(buildAccess(*StmtMatch->Result.Nodes.getNodeAs<Expr>("expr"),
+                          *StmtMatch->Result.Context),
+              ValueIs(std::string()));
+}
 } // namespace