arphaman updated this revision to Diff 113223.
arphaman added a comment.

Use `std::move`


Repository:
  rL LLVM

https://reviews.llvm.org/D37291

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===================================================================
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -35,8 +35,27 @@
 Expected<Optional<AtomicChanges>>
 createReplacements(const std::unique_ptr<RefactoringActionRule> &Rule,
                    RefactoringRuleContext &Context) {
-  return cast<SourceChangeRefactoringRule>(*Rule).createSourceReplacements(
-      Context);
+  class Consumer final : public RefactoringResultConsumer {
+    void handleInitiationFailure() {
+      Result = Expected<Optional<AtomicChanges>>(None);
+    }
+    void handleInitiationError(llvm::Error Err) { Result = std::move(Err); }
+
+    void handleInvocationError(llvm::Error Err) {
+      handleInitiationError(std::move(Err));
+    }
+
+    void handle(AtomicChanges SourceReplacements) {
+      Result = std::move(SourceReplacements);
+    }
+
+  public:
+    Optional<Expected<Optional<AtomicChanges>>> Result;
+  };
+
+  Consumer C;
+  Rule->invoke(C, Context);
+  return std::move(*C.Result);
 }
 
 TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
Index: include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
===================================================================
--- /dev/null
+++ include/clang/Tooling/Refactoring/RefactoringResultConsumer.h
@@ -0,0 +1,68 @@
+//===--- RefactoringResultConsumer.h - Clang refactoring library ----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace tooling {
+
+/// An abstract interface that consumes the various refactoring results that can
+/// be produced by refactoring actions.
+///
+/// A valid refactoring result must be handled by a \c handle method.
+class RefactoringResultConsumer {
+public:
+  virtual ~RefactoringResultConsumer() {}
+
+  /// Handles an initiation failure. An action typically fails to initiate when
+  /// the source selection has no overlap at all with any relevant AST nodes.
+  virtual void handleInitiationFailure() = 0;
+
+  /// Handles an initation error. The error typically has a \c DiagnosticError
+  /// payload that describes why initation failed.
+  virtual void handleInitiationError(llvm::Error Err) = 0;
+
+  virtual void handleInvocationError(llvm::Error Err) = 0;
+
+  /// Handles the source replacements that are produced by a refactoring action.
+  virtual void handle(AtomicChanges SourceReplacements) = 0;
+};
+
+namespace traits {
+namespace internal {
+
+template <typename T> struct HasHandle {
+private:
+  template <typename ClassT>
+  static auto check(ClassT *) -> typename std::is_same<
+      decltype(std::declval<ClassT>().handle(std::declval<T>())), void>::type;
+
+  template <typename> static std::false_type check(...);
+
+public:
+  using Type = decltype(check<RefactoringResultConsumer>(nullptr));
+};
+
+} // end namespace internal
+
+/// A type trait that returns true iff the given type is a valid refactoring
+/// result.
+template <typename T>
+struct IsValidRefactoringResult : internal::HasHandle<T>::Type {};
+
+} // end namespace traits
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RESULT_CONSUMER_H
Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
===================================================================
--- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
@@ -13,6 +13,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRule.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h"
+#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
 #include <type_traits>
@@ -22,39 +23,20 @@
 namespace refactoring_action_rules {
 namespace internal {
 
-/// A wrapper around a specific refactoring action rule that calls a generic
-/// 'perform' method from the specific refactoring method.
-template <typename T> struct SpecificRefactoringRuleAdapter {};
-
-template <>
-class SpecificRefactoringRuleAdapter<AtomicChanges>
-    : public SourceChangeRefactoringRule {
-public:
-  virtual Expected<Optional<AtomicChanges>>
-  perform(RefactoringRuleContext &Context) = 0;
-
-  Expected<Optional<AtomicChanges>>
-  createSourceReplacements(RefactoringRuleContext &Context) final override {
-    return perform(Context);
-  }
-};
-
 /// A specialized refactoring action rule that calls the stored function once
 /// all the of the requirements are fullfilled. The values produced during the
 /// evaluation of requirements are passed to the stored function.
-template <typename ResultType, typename FunctionType,
-          typename... RequirementTypes>
-class PlainFunctionRule final
-    : public SpecificRefactoringRuleAdapter<ResultType> {
+template <typename FunctionType, typename... RequirementTypes>
+class PlainFunctionRule final : public RefactoringActionRule {
 public:
   PlainFunctionRule(FunctionType Function,
                     std::tuple<RequirementTypes...> &&Requirements)
       : Function(Function), Requirements(std::move(Requirements)) {}
 
-  Expected<Optional<ResultType>>
-  perform(RefactoringRuleContext &Context) override {
-    return performImpl(Context,
-                       llvm::index_sequence_for<RequirementTypes...>());
+  void invoke(RefactoringResultConsumer &Consumer,
+              RefactoringRuleContext &Context) override {
+    return invokeImpl(Consumer, Context,
+                      llvm::index_sequence_for<RequirementTypes...>());
   }
 
 private:
@@ -95,22 +77,26 @@
   }
 
   template <size_t... Is>
-  Expected<Optional<ResultType>> performImpl(RefactoringRuleContext &Context,
-                                             llvm::index_sequence<Is...>) {
+  void invokeImpl(RefactoringResultConsumer &Consumer,
+                  RefactoringRuleContext &Context,
+                  llvm::index_sequence<Is...>) {
     // Initiate the operation.
     auto Values =
         std::make_tuple(std::get<Is>(Requirements).evaluate(Context)...);
     Optional<llvm::Error> InitiationFailure =
         findErrorNone(std::get<Is>(Values)...);
     if (InitiationFailure) {
       llvm::Error Error = std::move(*InitiationFailure);
       if (!Error)
-        return None;
-      return std::move(Error);
+        return Consumer.handleInitiationFailure();
+      return Consumer.handleInitiationError(std::move(Error));
     }
     // Perform the operation.
-    return Function(
-        unwrapRequirementResult(std::move(std::get<Is>(Values)))...);
+    auto Result =
+        Function(unwrapRequirementResult(std::move(std::get<Is>(Values)))...);
+    if (!Result)
+      return Consumer.handleInvocationError(Result.takeError());
+    Consumer.handle(std::move(*Result));
   }
 
   FunctionType Function;
Index: include/clang/Tooling/Refactoring/RefactoringActionRules.h
===================================================================
--- include/clang/Tooling/Refactoring/RefactoringActionRules.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRules.h
@@ -42,15 +42,12 @@
 createRefactoringRule(Expected<ResultType> (*RefactoringFunction)(
                           typename RequirementTypes::OutputType...),
                       const RequirementTypes &... Requirements) {
-  static_assert(
-      std::is_base_of<
-          RefactoringActionRule,
-          internal::SpecificRefactoringRuleAdapter<ResultType>>::value,
-      "invalid refactoring result type");
+  static_assert(tooling::traits::IsValidRefactoringResult<ResultType>::value,
+                "invalid refactoring result type");
   static_assert(traits::IsRequirement<RequirementTypes...>::value,
                 "invalid refactoring action rule requirement");
   return llvm::make_unique<internal::PlainFunctionRule<
-      ResultType, decltype(RefactoringFunction), RequirementTypes...>>(
+      decltype(RefactoringFunction), RequirementTypes...>>(
       RefactoringFunction, std::make_tuple(Requirements...));
 }
 
Index: include/clang/Tooling/Refactoring/RefactoringActionRule.h
===================================================================
--- include/clang/Tooling/Refactoring/RefactoringActionRule.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRule.h
@@ -11,52 +11,25 @@
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H
 
 #include "clang/Basic/LLVM.h"
-#include "clang/Tooling/Refactoring/AtomicChange.h"
-#include "llvm/Support/Error.h"
 #include <vector>
 
 namespace clang {
 namespace tooling {
 
+class RefactoringResultConsumer;
 class RefactoringRuleContext;
 
 /// A common refactoring action rule interface.
 class RefactoringActionRule {
 public:
-  enum RuleKind { SourceChangeRefactoringRuleKind };
-
-  RuleKind getRuleKind() const { return Kind; }
-
   virtual ~RefactoringActionRule() {}
 
-protected:
-  RefactoringActionRule(RuleKind Kind) : Kind(Kind) {}
-
-private:
-  RuleKind Kind;
-};
-
-/// A type of refactoring action rule that produces source replacements in the
-/// form of atomic changes.
-///
-/// This action rule is typically used for local refactorings that replace
-/// source in a single AST unit.
-class SourceChangeRefactoringRule : public RefactoringActionRule {
-public:
-  SourceChangeRefactoringRule()
-      : RefactoringActionRule(SourceChangeRefactoringRuleKind) {}
-
-  /// Initiates and performs a refactoring action that modifies the sources.
+  /// Initiates and performs a specific refactoring action.
   ///
-  /// The specific rule must return an llvm::Error with a DiagnosticError
-  /// payload or None when the refactoring action couldn't be initiated/
-  /// performed, or \c AtomicChanges when the action was performed successfully.
-  virtual Expected<Optional<AtomicChanges>>
-  createSourceReplacements(RefactoringRuleContext &Context) = 0;
-
-  static bool classof(const RefactoringActionRule *Rule) {
-    return Rule->getRuleKind() == SourceChangeRefactoringRuleKind;
-  }
+  /// The specific rule will invoke an appropriate \c handle method on a
+  /// consumer to propagate the result of the refactoring action.
+  virtual void invoke(RefactoringResultConsumer &Consumer,
+                      RefactoringRuleContext &Context) = 0;
 };
 
 /// A set of refactoring action rules that should have unique initiation
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to