[libTooling] Change Stencil equality to use `toString()`

[ymandel]

Summary

Removes the isEqual method from StencilPartInterface and modifies equality to
use the string representation returned by the toString method for comparison.

This means the run and selection stencils return true by default, and
clients should be cautious in relying on equality operator for comparison of
stencils containing parts generated by these functions.

It also means we no longer need the custom RTTI support, so it has been removed.

Patch by Harshal T. Lehri.

[ymandel]

removed operator== and updated tests accordingly.

clang/include/clang/Tooling/Transformer/Stencil.h

 public:
   virtual ~StencilPartInterface() = default;
 
   /// Evaluates this part to a string and appends it to \c Result.  \c Result is
   /// undefined in the case of an error.
   virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match,
                            std::string *Result) const = 0;
 
-  virtual bool isEqual(const StencilPartInterface &other) const = 0;
-
   /// Constructs a string representation of the StencilPart. StencilParts
   /// generated by the `selection` and `run` functions do not have a unique
   /// string representation.
   virtual std::string toString() const = 0;
 
-  const void *typeId() const { return TypeId; }
-
 protected:
-  StencilPartInterface(const void *DerivedId) : TypeId(DerivedId) {}
+  StencilPartInterface() = 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;
-
-  /// Unique identifier of the concrete type of this instance.  Supports safe
-  /// downcasting.
-  const void *TypeId;
 };
 
 /// 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);
   }
 
-  bool operator==(const StencilPart &Other) const {
-    if (Impl == Other.Impl)
-      return true;
-    if (Impl == nullptr || Other.Impl == nullptr)
-      return false;
-    return Impl->isEqual(*Other.Impl);
-  }
-
   std::string toString() const {
     if (Impl == nullptr)
       return "";
     return Impl->toString();
   }
 
 private:
   std::shared_ptr<StencilPartInterface> Impl;
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, ", ");
   }
 
 private:
-  friend bool operator==(const Stencil &A, const Stencil &B);
   static StencilPart wrap(llvm::StringRef Text);
   static StencilPart wrap(RangeSelector Selector);
   static StencilPart wrap(StencilPart Part) { return Part; }
 
   std::vector<StencilPart> Parts;
 };
 
-inline bool operator==(const Stencil &A, const Stencil &B) {
-  return A.Parts == B.Parts;
-}
-
-inline bool operator!=(const Stencil &A, const Stencil &B) { return !(A == B); }
-
 // Functions for conveniently building stencils.
 namespace stencil {
 /// 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)...);
 }
 
 /// \returns exactly the text provided.

clang/lib/Tooling/Transformer/Stencil.cpp

 
 using ast_matchers::MatchFinder;
 using ast_type_traits::DynTypedNode;
 using llvm::errc;
 using llvm::Error;
 using llvm::Expected;
 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);
-}
-
 static llvm::Expected<DynTypedNode>
 getNode(const ast_matchers::BoundNodes &Nodes, StringRef Id) {
   auto &NodesMap = Nodes.getMap();
   auto It = NodesMap.find(Id);
   if (It == NodesMap.end())
     return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
                                                "Id not bound: " + Id);
   return It->second;
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;
 };
 
-bool isEqualData(const RawTextData &A, const RawTextData &B) {
-  return A.Text == B.Text;
-}
-
-bool isEqualData(const DebugPrintNodeData &A, const DebugPrintNodeData &B) {
-  return A.Id == B.Id;
-}
-
-bool isEqualData(const UnaryOperationData &A, const UnaryOperationData &B) {
-  return A.Op == B.Op && A.Id == B.Id;
-}
-
-// Equality is not (yet) defined for \c RangeSelector.
-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;
-}
-
-// Equality is not defined over MatchConsumers, which are opaque.
-bool isEqualData(const MatchConsumer<std::string> &A,
-                 const MatchConsumer<std::string> &B) {
-  return false;
-}
-
 std::string toStringData(const RawTextData &Data) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << "\"";
   OS.write_escaped(Data.Text);
   OS << "\"";
   OS.flush();
   return Result;
case UnaryNodeOperator::Deref:
     break;
   case UnaryNodeOperator::Address:
     OpName = "addressOf";
     break;
   }
   return (OpName + "(\"" + Data.Id + "\")").str();
 }
 
-std::string toStringData(const SelectorData &) { return "SelectorData()"; }
+std::string toStringData(const SelectorData &) { return "selection(...)"; }
 
 std::string toStringData(const AccessData &Data) {
   return (llvm::Twine("access(\"") + Data.BaseId + "\", " +
           Data.Member.toString() + ")")
       .str();
 }
 
 std::string toStringData(const IfBoundData &Data) {
   return (llvm::Twine("ifBound(\"") + Data.Id + "\", " +
           Data.TruePart.toString() + ", " + Data.FalsePart.toString() + ")")
       .str();
 }
 
 std::string toStringData(const MatchConsumer<std::string> &) {
-  return "MatchConsumer<std::string>()";
+  return "run(...)";
 }
 
 // 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) {

 }
 
 template <typename T>
 class StencilPartImpl : public StencilPartInterface {
   T Data;
 
 public:
   template <typename... Ps>
-  explicit StencilPartImpl(Ps &&... Args)
-      : StencilPartInterface(StencilPartImpl::typeId()),
-        Data(std::forward<Ps>(Args)...) {}
-
-  // Generates a unique identifier for this class (specifically, one per
-  // instantiation of the template).
-  static const void* typeId() {
-    static bool b;
-    return &b;
-  }
+  explicit StencilPartImpl(Ps &&... Args) : Data(std::forward<Ps>(Args)...) {}
 
   Error eval(const MatchFinder::MatchResult &Match,
              std::string *Result) const override {
     return evalData(Data, Match, Result);
   }
 
-  bool isEqual(const StencilPartInterface &Other) const override {
-    if (const auto *OtherPtr = down_cast<StencilPartImpl>(&Other))
-      return isEqualData(Data, OtherPtr->Data);
-    return false;
-  }
-
   std::string toString() const override { return toStringData(Data); }
 };
 } // namespace
 
 StencilPart Stencil::wrap(StringRef Text) {
   return stencil::text(Text);
 }
 

clang/unittests/Tooling/StencilTest.cpp

TEST_F(StencilTest, RunOp) {
   StringRef Id = "id";
   auto SimpleFn = [Id](const MatchResult &R) {
     return std::string(R.Nodes.getNodeAs<Stmt>(Id) != nullptr ? "Bound"
                                                               : "Unbound");
   };
   testExpr(Id, "3;", cat(run(SimpleFn)), "Bound");
 }
 
-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"));
-  auto Rhs = cat(dPrint("node"), "foo");
-  EXPECT_NE(Lhs, Rhs);
-}
-
-TEST(StencilEqualityTest, InEqualityDifferentSizes) {
-  auto Lhs = cat("foo", dPrint("node"), "bar", "baz");
-  auto Rhs = cat("foo", dPrint("node"), "bar");
-  EXPECT_NE(Lhs, Rhs);
-}
-
-// node is opaque and therefore cannot be examined for equality.
-TEST(StencilEqualityTest, InEqualitySelection) {
-  auto S1 = cat(node("node"));
-  auto S2 = cat(node("node"));
-  EXPECT_NE(S1, S2);
-}
-
-// `run` is opaque.
-TEST(StencilEqualityTest, InEqualityRun) {
-  auto F = [](const MatchResult &R) { return "foo"; };
-  auto S1 = cat(run(F));
-  auto S2 = cat(run(F));
-  EXPECT_NE(S1, S2);
-}
-
 TEST(StencilToStringTest, RawTextOp) {
   auto S = cat("foo bar baz");
   StringRef Expected = R"("foo bar baz")";
   EXPECT_EQ(S.toString(), Expected);
 }
 
 TEST(StencilToStringTest, RawTextOpEscaping) {
   auto S = cat("foo \"bar\" baz\\n");

 }
 
 TEST(StencilToStringTest, AddressOfOp) {
   auto S = cat(addressOf("Id"));
   StringRef Expected = R"repr(addressOf("Id"))repr";
   EXPECT_EQ(S.toString(), Expected);
 }
 
+TEST(StencilToStringTest, SelectionOp) {
+  auto S1 = cat(node("node1"));
+  EXPECT_EQ(S1.toString(), "selection(...)");
+}
+
 TEST(StencilToStringTest, AccessOp) {
   auto S = cat(access("Id", text("memberData")));
   StringRef Expected = R"repr(access("Id", "memberData"))repr";
   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, IfBoundOp) {
   auto S = cat(ifBound("Id", text("trueText"), access("exprId", "memberData")));
   StringRef Expected =
       R"repr(ifBound("Id", "trueText", access("exprId", "memberData")))repr";
   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(...)");
+}
+
 TEST(StencilToStringTest, MultipleOp) {
   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);
 }
 } // namespace