ioeric updated this revision to Diff 49493.
ioeric added a comment.
- Added getOrCreateFileID interface in SourceManager.
http://reviews.llvm.org/D17761
Files:
include/clang/Basic/SourceManager.h
include/clang/Format/Format.h
include/clang/Tooling/Core/Replacement.h
lib/Format/CMakeLists.txt
lib/Format/Format.cpp
lib/Tooling/Core/Replacement.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11215,6 +11215,73 @@
EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces, Style));
}
+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(applyAllReplacementsAndFormat(Replaces, Context.Rewrite, Style));
+ EXPECT_EQ(Expected1, Context.getRewrittenText(ID1));
+ EXPECT_EQ(Expected2, Context.getRewrittenText(ID2));
+}
+
+TEST_F(ReplacementTest, ConflictReplacements) {
+ // Column limit is 20.
+ std::string Code1 = "Did you say it is a code?";
+
+ std::string Code2 = "Long *a =\n"
+ " new Long();\n"
+ "long x = 1;";
+ std::string Expected2 = "auto a = new Long();\n"
+ "long x =\n"
+ " 12345678901;";
+ FileID ID1 = Context.createInMemoryFile("fake.cpp", Code1);
+ FileID ID2 = Context.createInMemoryFile("real.cpp", Code2);
+ tooling::Replacements Replaces;
+
+ // Two Replacements for ID1 conflict with each other.
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID1, 1, 5), 6, "yoyoyo"));
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID1, 1, 3), 6, "yoyoyo?"));
+
+ // Replacements for ID2 should be applied independently of the failure in ID1.
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID2, 3, 10), 1, "12345678901"));
+ Replaces.insert(tooling::Replacement(
+ Context.Sources, Context.getLocation(ID2, 1, 1), 6, "auto "));
+
+ format::FormatStyle Style = format::getLLVMStyle();
+ Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
+ EXPECT_FALSE(applyAllReplacementsAndFormat(Replaces, Context.Rewrite, Style));
+ EXPECT_EQ(Expected2, Context.getRewrittenText(ID2));
+}
+
} // end namespace
} // end namespace format
} // end 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());
@@ -390,6 +384,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].push_back(Replace);
+ }
+ return FileToReplaces;
+}
+
Replacements mergeReplacements(const Replacements &First,
const Replacements &Second) {
if (First.empty() || Second.empty())
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,6 +1903,42 @@
return MergedReplacements;
}
+bool applyAllReplacementsAndFormat(const tooling::Replacements &Replaces,
+ Rewriter &Rewrite,
+ 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) {
+ const FileEntry *Entry = FileAndReplaces.first;
+ assert(Entry != nullptr && "Invalid file entry!");
+ auto &Repls = FileAndReplaces.second;
+
+ std::vector<tooling::Range> Conflicts;
+ tooling::deduplicate(Repls, Conflicts);
+
+ if (!Conflicts.empty()) {
+ Result = false;
+ continue;
+ }
+
+ FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
+ StringRef Code = SM.getBufferData(ID);
+
+ // FIXME: remove this when `Replacements` is implemented as `std::vector`.
+ tooling::Replacements R;
+ std::copy(Repls.begin(), Repls.end(), inserter(R, R.begin()));
+ tooling::Replacements NewReplacements =
+ formatReplacements(Code, R, Style);
+ Result = applyAllReplacements(NewReplacements, Rewrite) && Result;
+ }
+ return Result;
+}
+
std::string applyAllReplacementsAndFormat(StringRef Code,
const tooling::Replacements &Replaces,
const FormatStyle &Style) {
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/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,13 +223,25 @@
/// 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);
+// FIXME: change the map value to be `Replacements` instead of
+// 'std::vector<Replacement>' when `Replacements` is implemented as
+// `std::vector`. Since `deduplicate` takes `std::vector<Replacement>` as
+// input, we need to use vector.
+typedef std::map<const FileEntry *, std::vector<Replacement>>
+ 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',
/// replacements must not overlap.
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -737,6 +737,22 @@
const tooling::Replacements &Replaces,
const FormatStyle &Style);
+/// \brief Groups \p Replaces by the file path and applies each group of
+/// Replacements on the related file in \p Rewriter. In addition to applying
+/// given Replacements, this function also formats the changed code.
+///
+/// \pre Replacements must be conflict-free.
+///
+/// Replacement applications happen independently of the success of other
+/// applications.
+///
+/// \returns true if all replacements applied and formatted. false otherwise.
+///
+/// See also "clang/Tooling/Core/Replacements.h".
+bool applyAllReplacementsAndFormat(const tooling::Replacements &Replaces,
+ Rewriter &Rewrite,
+ const format::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.
///
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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
