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

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 24 05:05:31 PDT 2017

arphaman added inline comments.

Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template <typename T>
+    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?



More information about the cfe-commits mailing list