[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D38982#900744, @ioeric wrote:

> Code looks good in general. I see there are a bunch of major features missing; it might make sense to print a warning or document the major missing features in the high-level API.


I added a warning in the description of the action.



================
Comment at: include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:75
 /// \see CodeRangeASTSelection
-class CodeRangeSelectionRequirement : public ASTSelectionRequirement {
+class CodeRangeASTSelectionRequirement : public ASTSelectionRequirement {
 public:
----------------
Renamed to `CodeRangeASTSelectionRequirement`


================
Comment at: lib/Tooling/Refactoring/Extract.cpp:167
+      OS << "return ";
+    OS << ExtractedCodeRewriter.getRewrittenText(ExtractedRange);
+    // FIXME: Compute the correct semicolon policy.
----------------
ioeric wrote:
> An alternative way to get the source code is:
> ``` 
> Lexer::getSourceText(
>       CharSourceRange::getTokenRange(SM.getSpellingLoc(Start), SM.getSpellingLoc(End)),
>       SM, Result.Context->getLangOpts());
> ```
I will need to get the rewritten source, so I've started using it in the first patch.


================
Comment at: tools/clang-refactor/TestSupport.cpp:106
+    auto Buf = llvm::MemoryBuffer::getMemBuffer(Result->c_str());
+    for (llvm::line_iterator It(*Buf, /*SkipBlanks=*/false); !It.is_at_end();
+         ++It) {
----------------
ioeric wrote:
> Can we filter the `CHECK`s with `sed` in the test? Similar to https://github.com/llvm-mirror/clang-tools-extra/blob/master/test/change-namespace/simple-move.cpp#L1
Fixed.


Repository:
  rL LLVM

https://reviews.llvm.org/D38982