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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits