[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template <typename T>
+detail::SourceSelectionRequirement<
+    typename selection::detail::EvaluateSelectionChecker<
----------------
ioeric wrote:
> Could you help me understand this class? 
> 
> This seems to be a selection-specific requirement and should live in `selection`. It is also used in `BaseSpecializedRule` which seems to be a higher level of abstraction. 
It's just a container class that stores all information about the `requiredSelection` requirement. I agree about `BaseSpecializedRule`, that connection should be chopped. I will move the evaluation code into the requirement itself when I update the patch.

I would tend to disagree about moving it though, as `SourceSelectionRequirement` is a requirement first and I think that's why it should live with other requirements. Yes, it's related to selections, but it uses them to implement the requirement. I think it's better to keep requirements together, as opposed to having `option` requirements close to options, selection requirements close to selection, and so on. WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D36075