ioeric updated this revision to Diff 49620.
ioeric added a comment.
- Removed definitions that were forgotten.
http://reviews.llvm.org/D17761
Files:
include/clang/Basic/SourceManager.h
include/clang/Format/Format.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/ReplacementsFormat.h
lib/Format/CMakeLists.txt
lib/Format/Format.cpp
lib/Tooling/CMakeLists.txt
lib/Tooling/Core/Replacement.cpp
lib/Tooling/ReplacementsFormat.cpp
unittests/Format/FormatTest.cpp
unittests/Tooling/CMakeLists.txt
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===================================================================
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -18,11 +18,13 @@
#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"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/ReplacementsFormat.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/Path.h"
@@ -166,6 +168,42 @@
EXPECT_EQ("z", Context.getRewrittenText(IDz));
}
+TEST_F(ReplacementTest, MultipleFilesReplaceAndFormat) {
+ // Column limit is 20.
+ std::string Code1 = "Long *a =\n"
+ " new Long();\n"
+ "long x = 1;";
+ std::string Expected1 = "auto a = new Long();\n"
+ "long x =\n"
+ " 12345678901;";
+ std::string Code2 = "int x = 123;\n"
+ "int y = 0;";
+ std::string Expected2 = "int x =\n"
+ " 1234567890123;\n"
+ "int y = 10;";
+ FileID ID1 = Context.createInMemoryFile("format_1.cpp", Code1);
+ FileID ID2 = Context.createInMemoryFile("format_2.cpp", Code2);
+
+ tooling::Replacements Replaces;
+ // Scrambled the order of replacements.
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123"));
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto "));
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10"));
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID1, 3, 10), 1, "12345678901"));
+
+ format::FormatStyle Style = format::getLLVMStyle();
+ Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
+
+ EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+ EXPECT_TRUE(formatRewrittenCode(Context.Rewrite, Replaces, Style));
+ EXPECT_EQ(Expected1, Context.getRewrittenText(ID1));
+ EXPECT_EQ(Expected2, Context.getRewrittenText(ID2));
+}
+
TEST(ShiftedCodePositionTest, FindsNewCodePosition) {
Replacements Replaces;
Replaces.insert(Replacement("", 0, 1, ""));
Index: unittests/Tooling/CMakeLists.txt
===================================================================
--- unittests/Tooling/CMakeLists.txt
+++ unittests/Tooling/CMakeLists.txt
@@ -24,6 +24,7 @@
clangAST
clangASTMatchers
clangBasic
+ clangFormat
clangFrontend
clangLex
clangRewrite
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11212,7 +11212,8 @@
format::FormatStyle Style = format::getLLVMStyle();
Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
- EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces, Style));
+ EXPECT_EQ(Expected, applyAllReplacements(
+ Code, formatReplacements(Code, Replaces, Style)));
}
} // end namespace
Index: lib/Tooling/ReplacementsFormat.cpp
===================================================================
--- /dev/null
+++ lib/Tooling/ReplacementsFormat.cpp
@@ -0,0 +1,75 @@
+//===--- ReplacementFormat.cpp - Format code changed by replacemnts -------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// \file
+// \brief This file implements functions that reformat code changed by
+// replacements.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Tooling/ReplacementsFormat.h"
+
+#include "clang/Basic/SourceManager.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief Reformat code changed by \p Replaces in \p Rewrite.
+bool formatRewrittenCode(Rewriter &Rewrite, const Replacements &Replaces,
+ const format::FormatStyle &Style) {
+ SourceManager &SM = Rewrite.getSourceMgr();
+ FileManager &Files = SM.getFileManager();
+
+ tooling::FileToReplacementsMap FileToReplaces =
+ groupReplacementsByFile(Replaces, Files);
+
+ bool Result = true;
+ for (auto &FileAndReplaces : FileToReplaces) {
+ std::vector<Range> Ranges =
+ calculateChangedRangesInFile(FileAndReplaces.second);
+
+ const FileEntry *Entry = FileAndReplaces.first;
+ assert(Entry != nullptr && "Invalid file entry!");
+
+ FileID ID = SM.translateFile(Entry);
+ // ID must be valid since `Replaces` have applied.
+ assert(ID.isValid() && "Expect valid FileID!");
+
+ // Retrive the changed code from `Rewrite`, reformat the retrieved code, and
+ // override the `RewriteBuffer` corresponding to the current file in
+ // `Rewrite`.
+ // FIXME: this seems to be the only way to work on a modified
+ // `RewriteBuffer` since the changes of the replacements do not reflect on
+ // the `SourceManager`. Even if we override content of the file entry in
+ // `SourceManager`, it still doesn't work since Source Location's ID will
+ // become inconsistent. To cope with that, we need to create a new FileID
+ // for the overriden file entry, which will leave 2 FileIDs for the same
+ // `FileEntry` in the `SourceManager`. But when we reference FileID for the
+ // same `FileEntry` in `applyAllReplacements`, the current implementation
+ // will get the old FileID(the first occurrence), which means that the
+ // inconsistency of SourceLocation IDs still exists.
+ auto *Buffer = Rewrite.getRewriteBufferFor(ID);
+ std::string Code(Buffer->begin(), Buffer->end());
+
+ Replacements NewReplaces =
+ format::reformat(Style, Code, Ranges, Entry->getName());
+ std::string NewCode = applyAllReplacements(Code, NewReplaces);
+ if (NewCode.empty()) {
+ Result = false;
+ continue;
+ }
+ Rewrite.getEditBuffer(ID).Initialize(NewCode);
+ }
+ return Result;
+}
+
+} // namespace tooling
+} // namespace clang
Index: lib/Tooling/Core/Replacement.cpp
===================================================================
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -58,14 +58,8 @@
const FileEntry *Entry = SM.getFileManager().getFile(FilePath);
if (!Entry)
return false;
- FileID ID;
- // FIXME: Use SM.translateFile directly.
- SourceLocation Location = SM.translateFileLineCol(Entry, 1, 1);
- ID = Location.isValid() ?
- SM.getFileID(Location) :
- SM.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
- // FIXME: We cannot check whether Offset + Length is in the file, as
- // the remapping API is not public in the RewriteBuffer.
+
+ FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
const SourceLocation Start =
SM.getLocForStartOfFile(ID).
getLocWithOffset(ReplacementRange.getOffset());
@@ -256,6 +250,8 @@
}
std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
+ if (Replaces.empty()) return Code;
+
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
new vfs::InMemoryFileSystem);
FileManager Files(FileSystemOptions(), InMemoryFileSystem);
@@ -390,6 +386,17 @@
};
} // namespace
+FileToReplacementsMap
+groupReplacementsByFile(const Replacements &Replaces, FileManager &Files) {
+ FileToReplacementsMap FileToReplaces;
+ for (const auto &Replace : Replaces) {
+ const FileEntry *Entry = Files.getFile(Replace.getFilePath());
+ assert(Entry && "Expected an existing file.");
+ FileToReplaces[Entry].insert(Replace);
+ }
+ return FileToReplaces;
+}
+
Replacements mergeReplacements(const Replacements &First,
const Replacements &Second) {
if (First.empty() || Second.empty())
Index: lib/Tooling/CMakeLists.txt
===================================================================
--- lib/Tooling/CMakeLists.txt
+++ lib/Tooling/CMakeLists.txt
@@ -10,14 +10,16 @@
JSONCompilationDatabase.cpp
Refactoring.cpp
RefactoringCallbacks.cpp
+ ReplacementsFormat.cpp
Tooling.cpp
LINK_LIBS
clangAST
clangASTMatchers
clangBasic
clangDriver
clangFrontend
+ clangFormat
clangLex
clangRewrite
clangToolingCore
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -23,6 +23,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
+#include "clang/Rewrite/Core/Rewriter.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
@@ -1902,16 +1903,6 @@
return MergedReplacements;
}
-std::string applyAllReplacementsAndFormat(StringRef Code,
- const tooling::Replacements &Replaces,
- const FormatStyle &Style) {
- tooling::Replacements NewReplacements =
- formatReplacements(Code, Replaces, Style);
- if (NewReplacements.empty())
- return Code; // Exit early to avoid overhead in `applyAllReplacements`.
- return applyAllReplacements(Code, NewReplacements);
-}
-
tooling::Replacements reformat(const FormatStyle &Style,
SourceManager &SourceMgr, FileID ID,
ArrayRef<CharSourceRange> Ranges,
Index: lib/Format/CMakeLists.txt
===================================================================
--- lib/Format/CMakeLists.txt
+++ lib/Format/CMakeLists.txt
@@ -13,5 +13,6 @@
LINK_LIBS
clangBasic
clangLex
+ clangRewrite
clangToolingCore
)
Index: include/clang/Tooling/ReplacementsFormat.h
===================================================================
--- /dev/null
+++ include/clang/Tooling/ReplacementsFormat.h
@@ -0,0 +1,40 @@
+//===-- ReplacementsFormat.h -- Format code changed by replacements -*- C++ -*//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file defines functions that reformat code changed by
+/// replacemnts.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H
+#define LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H
+
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief Reformat code changed by \p Replaces in \p Rewrite.
+///
+/// \pre \p Replaces must have applied on \p Rewrite, and no other replacements
+/// apply after that. Also, \p Replaces must be conflict-free.
+///
+/// Code reformatting happens independently of the success of other files.
+///
+/// \returns true if we successfully reformatted all changed code in \p Rewrite.
+/// false otherwise.
+bool formatRewrittenCode(Rewriter &Rewrite, const Replacements &Replaces,
+ const format::FormatStyle &Style);
+
+} // namespace tooling
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H
Index: include/clang/Tooling/Core/Replacement.h
===================================================================
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -22,12 +22,15 @@
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/StringRef.h"
+#include <map>
#include <set>
#include <string>
#include <vector>
namespace clang {
+class FileEntry;
+class FileManager;
class Rewriter;
namespace tooling {
@@ -220,12 +223,19 @@
/// replacements cannot be applied, this returns an empty \c string.
std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
-/// \brief Calculate the ranges in a single file that are affected by the
+/// \brief Calculates 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);
+std::vector<Range> calculateChangedRangesInFile(const Replacements &Replaces);
+
+typedef std::map<const FileEntry *, Replacements>
+ FileToReplacementsMap;
+
+/// \brief Groups a random set of replacements by file path. Replacements
+/// related to the same file entry are put into the same vector.
+FileToReplacementsMap groupReplacementsByFile(const Replacements &Replaces,
+ FileManager &Files);
/// \brief Merges two sets of replacements with the second set referring to the
/// code after applying the first set. Within both 'First' and 'Second',
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -737,22 +737,6 @@
const tooling::Replacements &Replaces,
const FormatStyle &Style);
-/// \brief In addition to applying all replacements in \p Replaces to \p Code,
-/// 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 with all replacements applied and formatted, if
-/// successful. An empty string otherwise.
-///
-/// See also "include/clang/Tooling/Core/Replacements.h".
-std::string applyAllReplacementsAndFormat(StringRef Code,
- const tooling::Replacements &Replaces,
- const FormatStyle &Style);
-
/// \brief Reformats the given \p Ranges in the file \p ID.
///
/// Each range is extended on either end to its next bigger logic unit, i.e.
Index: include/clang/Basic/SourceManager.h
===================================================================
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -797,6 +797,15 @@
IncludeLoc, FileCharacter, LoadedID, LoadedOffset);
}
+ /// \brief Get the FileID for \p SourceFile if it exists. Otherwise, create a
+ /// new FileID for the \p SourceFile.
+ FileID getOrCreateFileID(const FileEntry *SourceFile,
+ SrcMgr::CharacteristicKind FileCharacter) {
+ FileID ID = translateFile(SourceFile);
+ return ID.isValid() ? ID : createFileID(SourceFile, SourceLocation(),
+ FileCharacter);
+ }
+
/// \brief Return a new SourceLocation that encodes the
/// fact that a token from SpellingLoc should actually be referenced from
/// ExpansionLoc, and that it represents the expansion of a macro argument
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
