r262232 - Implement new interfaces for code-formatting when applying replacements.
Chandler Carruth via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 29 08:39:37 PST 2016
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.
>
> 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/4051004b/attachment-0001.html>
More information about the cfe-commits
mailing list