Avoid extra copy when applying r-value Patch

Summary:
Currently `apply` is a const method, which would copy the data. If we can discard the patch after applying, we can move the data instead.

This diff optimized for StringPatch, eventually we want to use the same technique to optimize for other patches.

Reviewed By: thedavekwon

Differential Revision: D63044259

fbshipit-source-id: f73859ec558747c8cb66be165d2723cfd464e832

thrift/lib/cpp2/op/detail/BasePatch.h

@@ -263,18 +263,23 @@ class BaseClearPatch : public BaseAssignPatch<Patch, Derived> {
   using Base::resetAnd;
   ~BaseClearPatch() = default;
 
-  template <typename Visitor>
-  bool customVisitAssignAndClear(Visitor&& v) const {
-    if (hasAssign()) {
-      std::forward<Visitor>(v).assign(*data_.assign());
+ private:
+  template <typename Self, typename Visitor>
+  static bool customVisitAssignAndClearImpl(Self&& self, Visitor&& v) {
+    if (std::forward<Self>(self).hasAssign()) {
+      std::forward<Visitor>(v).assign(*std::forward<Self>(self).data_.assign());
       return true;
     }
-    if (data_.clear() == true) {
+    if (std::forward<Self>(self).data_.clear() == true) {
       std::forward<Visitor>(v).clear();
     }
     return false;
   }
 
+ protected:
+  FOLLY_FOR_EACH_THIS_OVERLOAD_IN_CLASS_BODY_DELEGATE(
+      customVisitAssignAndClear, customVisitAssignAndClearImpl);
+
   /// Clears the value.
   void clear() { resetAnd().clear() = true; }
 };

thrift/lib/cpp2/op/detail/ValuePatch.h

@@ -234,6 +234,7 @@ class BaseStringPatch : public BaseContainerPatch<Patch, Derived> {
     return derived();
   }
 
+ private:
   /// @copybrief AssignPatch::customVisit
   ///
   /// Users should provide a visitor with the following methods
@@ -254,21 +255,27 @@ class BaseStringPatch : public BaseContainerPatch<Patch, Derived> {
   ///
   ///     v.prepend("(");
   ///     v.append(")");
-  template <class Visitor>
-  void customVisit(Visitor&& v) const {
+  template <class Self, class Visitor>
+  static void customVisitImpl(Self&& self, Visitor&& v) {
     if (false) {
       // Test whether the required methods exist in Visitor
       v.assign(T{});
       v.clear();
       v.prepend(T{});
       v.append(T{});
     }
-    if (!Base::template customVisitAssignAndClear(std::forward<Visitor>(v))) {
-      std::forward<Visitor>(v).prepend(*data_.prepend());
-      std::forward<Visitor>(v).append(*data_.append());
+    if (!std::forward<Self>(self).customVisitAssignAndClear(
+            std::forward<Visitor>(v))) {
+      std::forward<Visitor>(v).prepend(
+          *std::forward<Self>(self).data_.prepend());
+      std::forward<Visitor>(v).append(*std::forward<Self>(self).data_.append());
     }
   }
 
+ public:
+  FOLLY_FOR_EACH_THIS_OVERLOAD_IN_CLASS_BODY_DELEGATE(
+      customVisit, customVisitImpl);
+
  protected:
   using Base::assignOr;
   using Base::data_;
@@ -319,19 +326,26 @@ class StringPatch : public BaseStringPatch<Patch, StringPatch<Patch>> {
     cur = std::forward<U>(val) + std::move(cur);
   }
 
-  void apply(T& val) const {
+ private:
+  template <class Self>
+  static void applyImpl(Self&& self, T& val) {
     struct Visitor {
       T& v;
       void assign(const T& t) { v = t; }
-      void clear() { v = ""; }
-      // TODO: Optimize this
+      void assign(T&& t) { v = std::move(t); }
+      void clear() { v.clear(); }
       void prepend(const T& t) { v = t + v; }
+      void prepend(T&& t) { v = std::move(t) + v; }
       void append(const T& t) { v += t; }
+      void append(T&& t) { v += std::move(t); }
     };
 
-    return Base::customVisit(Visitor{val});
+    return std::forward<Self>(self).customVisit(Visitor{val});
   }
 
+ public:
+  FOLLY_FOR_EACH_THIS_OVERLOAD_IN_CLASS_BODY_DELEGATE(apply, applyImpl);
+
  private:
   using Base::assignOr;
   using Base::data_;