r262232 - Implement new interfaces for code-formatting when applying replacements.
Manuel Klimek via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 29 08:49:35 PST 2016
On Mon, Feb 29, 2016 at 5:39 PM Chandler Carruth <chandlerc at gmail.com>
wrote:
> On Mon, Feb 29, 2016 at 11:32 AM Manuel Klimek via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: klimek
>> Date: Mon Feb 29 10:27:41 2016
>> New Revision: 262232
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=262232&view=rev
>> Log:
>> Implement new interfaces for code-formatting when applying replacements.
>>
>
> Random request: provide more context in commit logs. =D
>
> When I randomly end up staring at patches for some reason, its really
> useful to know what the background is, or at least a bit of a clue.
>
> For example, here, why does applying replacements need new interfaces?
>
> Specifically context for those not 100% actively following along with the
> detailed discussion are super useful in the commit log where others end up
> reading about changes.
>
Thanks. Btw, this particular change has been rolled back because it
violates layering (that I completely missed).
>
>
>>
>> Patch by Eric Liu.
>>
>> Modified:
>> cfe/trunk/include/clang/Tooling/Core/Replacement.h
>> cfe/trunk/lib/Tooling/Core/Replacement.cpp
>> cfe/trunk/unittests/Tooling/CMakeLists.txt
>> cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>>
>> Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=262232&r1=262231&r2=262232&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
>> +++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Mon Feb 29
>> 10:27:41 2016
>> @@ -30,6 +30,10 @@ namespace clang {
>>
>> class Rewriter;
>>
>> +namespace format {
>> +struct FormatStyle;
>> +} // namespace format
>> +
>> namespace tooling {
>>
>> /// \brief A source range independent of the \c SourceManager.
>> @@ -220,6 +224,41 @@ bool applyAllReplacements(const std::vec
>> /// replacements cannot be applied, this returns an empty \c string.
>> std::string applyAllReplacements(StringRef Code, const Replacements
>> &Replaces);
>>
>> +/// \brief Applies all replacements in \p Replaces to \p Code.
>> +///
>> +/// This completely ignores the path stored in each replacement. If one
>> or more
>> +/// replacements cannot be applied, this returns an empty \c string.
>> +std::string applyAllReplacements(StringRef Code,
>> + const std::vector<Replacements>
>> &Replaces);
>> +
>> +/// \brief Calculate the ranges in a single file that are affected by the
>> +/// Replacements.
>> +///
>> +/// \pre Replacements must be for the same file.
>> +std::vector<tooling::Range>
>> +calculateChangedRangesInFile(const tooling::Replacements &Replaces);
>> +
>> +/// \brief Return replacements that are merged from orginal replacements
>> +/// and the replacements for formatting the code after applying the
>> orginal
>> +/// replacements.
>> +tooling::Replacements formatReplacements(StringRef Code,
>> + const tooling::Replacements
>> &Replaces,
>> + const format::FormatStyle
>> &Style);
>> +
>> +/// \brief In addition to applying replacements as in
>> `applyAllReplacements`,
>> +/// this function also reformats the changed code after applying
>> replacements.
>> +///
>> +/// \pre Replacements must be for the same file and conflict-free.
>> +///
>> +/// Replacement applications happen independently of the success of
>> +/// other applications.
>> +///
>> +/// \returns the changed code if all replacements apply and code is
>> fixed.
>> +/// empty string otherwise.
>> +std::string applyAllReplacementsAndFormat(StringRef Code,
>> + const Replacements &Replaces,
>> + const format::FormatStyle
>> &Style);
>> +
>> /// \brief Merges two sets of replacements with the second set referring
>> to the
>> /// code after applying the first set. Within both 'First' and 'Second',
>> /// replacements must not overlap.
>>
>> Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=262232&r1=262231&r2=262232&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
>> +++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Mon Feb 29 10:27:41 2016
>> @@ -11,17 +11,20 @@
>> //
>>
>> //===----------------------------------------------------------------------===//
>>
>> +#include "clang/Tooling/Core/Replacement.h"
>> +
>> #include "clang/Basic/Diagnostic.h"
>> #include "clang/Basic/DiagnosticIDs.h"
>> #include "clang/Basic/DiagnosticOptions.h"
>> #include "clang/Basic/FileManager.h"
>> #include "clang/Basic/SourceManager.h"
>> +#include "clang/Format/Format.h"
>> #include "clang/Lex/Lexer.h"
>> #include "clang/Rewrite/Core/Rewriter.h"
>> -#include "clang/Tooling/Core/Replacement.h"
>> #include "llvm/Support/FileSystem.h"
>> #include "llvm/Support/Path.h"
>> #include "llvm/Support/raw_os_ostream.h"
>> +#include <iterator>
>>
>> namespace clang {
>> namespace tooling {
>> @@ -281,6 +284,44 @@ std::string applyAllReplacements(StringR
>> return Result;
>> }
>>
>> +tooling::Replacements formatReplacements(StringRef Code,
>> + const tooling::Replacements
>> &Replaces,
>> + const format::FormatStyle
>> &Style) {
>> + if (Replaces.empty()) return Replacements();
>> +
>> + std::string NewCode = applyAllReplacements(Code, Replaces);
>> + std::vector<tooling::Range> ChangedRanges =
>> + calculateChangedRangesInFile(Replaces);
>> + StringRef FileName = Replaces.begin()->getFilePath();
>> + tooling::Replacements FormatReplaces =
>> + format::reformat(Style, NewCode, ChangedRanges, FileName);
>> +
>> + tooling::Replacements MergedReplacements =
>> + mergeReplacements(Replaces, FormatReplaces);
>> + return MergedReplacements;
>> +}
>> +
>> +std::vector<Range> calculateChangedRangesInFile(const Replacements
>> &Replaces) {
>> + std::vector<Range> ChangedRanges;
>> + int Shift = 0;
>> + for (const tooling::Replacement &R : Replaces) {
>> + unsigned Offset = R.getOffset() + Shift;
>> + unsigned Length = R.getReplacementText().size();
>> + Shift += Length - R.getLength();
>> + ChangedRanges.push_back(tooling::Range(Offset, Length));
>> + }
>> + return ChangedRanges;
>> +}
>> +
>> +std::string applyAllReplacementsAndFormat(StringRef Code,
>> + const Replacements &Replaces,
>> + const format::FormatStyle
>> &Style) {
>> + Replacements NewReplacements = formatReplacements(Code, Replaces,
>> Style);
>> + if (NewReplacements.empty())
>> + return Code; // Exit early to avoid overhead in
>> `applyAllReplacements`.
>> + return applyAllReplacements(Code, NewReplacements);
>> +}
>> +
>> namespace {
>> // Represents a merged replacement, i.e. a replacement consisting of
>> multiple
>> // overlapping replacements from 'First' and 'Second' in
>> mergeReplacements.
>> @@ -314,7 +355,7 @@ public:
>>
>> // Merges the next element 'R' into this merged element. As we always
>> merge
>> // from 'First' into 'Second' or vice versa, the MergedReplacement
>> knows what
>> - // set the next element is coming from.
>> + // set the next element is coming from.
>> void merge(const Replacement &R) {
>> if (MergeSecond) {
>> unsigned REnd = R.getOffset() + Delta + R.getLength();
>>
>> Modified: cfe/trunk/unittests/Tooling/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CMakeLists.txt?rev=262232&r1=262231&r2=262232&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Tooling/CMakeLists.txt (original)
>> +++ cfe/trunk/unittests/Tooling/CMakeLists.txt Mon Feb 29 10:27:41 2016
>> @@ -24,6 +24,7 @@ target_link_libraries(ToolingTests
>> clangAST
>> clangASTMatchers
>> clangBasic
>> + clangFormat
>> clangFrontend
>> clangLex
>> clangRewrite
>>
>> Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=262232&r1=262231&r2=262232&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
>> +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Mon Feb 29 10:27:41
>> 2016
>> @@ -18,6 +18,7 @@
>> #include "clang/Basic/FileManager.h"
>> #include "clang/Basic/LangOptions.h"
>> #include "clang/Basic/SourceManager.h"
>> +#include "clang/Format/Format.h"
>> #include "clang/Frontend/CompilerInstance.h"
>> #include "clang/Frontend/FrontendAction.h"
>> #include "clang/Frontend/TextDiagnosticPrinter.h"
>> @@ -166,6 +167,35 @@ TEST_F(ReplacementTest, ApplyAllFailsIfO
>> EXPECT_EQ("z", Context.getRewrittenText(IDz));
>> }
>>
>> +TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
>> + // Column limit is 20.
>> + std::string Code = "Type *a =\n"
>> + " new Type();\n"
>> + "g(iiiii, 0, jjjjj,\n"
>> + " 0, kkkkk, 0, mm);\n"
>> + "int bad = format ;";
>> + std::string Expected = "auto a = new Type();\n"
>> + "g(iiiii, nullptr,\n"
>> + " jjjjj, nullptr,\n"
>> + " kkkkk, nullptr,\n"
>> + " mm);\n"
>> + "int bad = format ;";
>> + FileID ID = Context.createInMemoryFile("format.cpp", Code);
>> + Replacements Replaces;
>> + Replaces.insert(
>> + Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 6,
>> "auto "));
>> + Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID,
>> 3, 10),
>> + 1, "nullptr"));
>> + Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID,
>> 4, 3), 1,
>> + "nullptr"));
>> + Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID,
>> 4, 13),
>> + 1, "nullptr"));
>> +
>> + format::FormatStyle Style = format::getLLVMStyle();
>> + Style.ColumnLimit = 20; // Set column limit to 20 to increase
>> readibility.
>> + EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces,
>> Style));
>> +}
>> +
>> TEST(ShiftedCodePositionTest, FindsNewCodePosition) {
>> Replacements Replaces;
>> Replaces.insert(Replacement("", 0, 1, ""));
>> @@ -418,6 +448,25 @@ TEST(Range, contains) {
>> EXPECT_FALSE(Range(0, 10).contains(Range(0, 11)));
>> }
>>
>> +TEST(Range, CalculateRangesOfReplacements) {
>> + // Before: aaaabbbbbbz
>> + // After : bbbbbbzzzzzzoooooooooooooooo
>> + Replacements Replaces;
>> + Replaces.insert(Replacement("foo", 0, 4, ""));
>> + Replaces.insert(Replacement("foo", 10, 1, "zzzzzz"));
>> + Replaces.insert(Replacement("foo", 11, 0, "oooooooooooooooo"));
>> +
>> + std::vector<Range> Ranges = calculateChangedRangesInFile(Replaces);
>> +
>> + EXPECT_EQ(3ul, Ranges.size());
>> + EXPECT_TRUE(Ranges[0].getOffset() == 0);
>> + EXPECT_TRUE(Ranges[0].getLength() == 0);
>> + EXPECT_TRUE(Ranges[1].getOffset() == 6);
>> + EXPECT_TRUE(Ranges[1].getLength() == 6);
>> + EXPECT_TRUE(Ranges[2].getOffset() == 12);
>> + EXPECT_TRUE(Ranges[2].getLength() == 16);
>> +}
>> +
>> TEST(DeduplicateTest, removesDuplicates) {
>> std::vector<Replacement> Input;
>> Input.push_back(Replacement("fileA", 50, 0, " foo "));
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160229/d908841b/attachment-0001.html>
More information about the cfe-commits
mailing list