ioeric updated this revision to Diff 60680.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- addressed reviewer's comments.
http://reviews.llvm.org/D21323
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -688,6 +688,29 @@
EXPECT_EQ(Expected, apply(Code, Replaces));
}
+TEST_F(CleanUpReplacementsTest, SkipExistingHeaders) {
+ std::string Code = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ std::string Expected = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>"),
+ createInsertion("#include \"a.h\"")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, AddIncludesWithDifferentForms) {
+ std::string Code = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ // FIXME: this might not be the best behavior.
+ std::string Expected = "#include \"a.h\"\n"
+ "#include \"vector\"\n"
+ "#include <vector>\n"
+ "#include <a.h>\n";
+ tooling::Replacements Replaces = {createInsertion("#include \"vector\""),
+ createInsertion("#include <a.h>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1486,7 +1486,6 @@
// code.
// FIXME: do not insert headers into conditional #include blocks, e.g.
#includes
// surrounded by compile condition "#if...".
-// FIXME: do not insert existing headers.
// FIXME: insert empty lines between newly created blocks.
tooling::Replacements
fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
@@ -1533,10 +1532,12 @@
TrimmedCode.split(Lines, '\n');
unsigned Offset = MinInsertOffset;
unsigned NextLineOffset;
+ std::set<StringRef> ExistingIncludes;
for (auto Line : Lines) {
NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
if (IncludeRegex.match(Line, &Matches)) {
StringRef IncludeName = Matches[2];
+ ExistingIncludes.insert(IncludeName);
int Category = Categories.getIncludePriority(
IncludeName, /*CheckMainHeader=*/FirstIncludeOffset < 0);
CategoryEndOffsets[Category] = NextLineOffset;
@@ -1572,6 +1573,11 @@
"'#include ...'");
(void)Matched;
auto IncludeName = Matches[2];
+ if (ExistingIncludes.find(IncludeName) != ExistingIncludes.end()) {
+ DEBUG(llvm::dbgs() << "Skip adding existing include : " << IncludeName
+ << "\n");
+ continue;
+ }
int Category =
Categories.getIncludePriority(IncludeName, /*CheckMainHeader=*/true);
Offset = CategoryEndOffsets[Category];
Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -688,6 +688,29 @@
EXPECT_EQ(Expected, apply(Code, Replaces));
}
+TEST_F(CleanUpReplacementsTest, SkipExistingHeaders) {
+ std::string Code = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ std::string Expected = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>"),
+ createInsertion("#include \"a.h\"")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, AddIncludesWithDifferentForms) {
+ std::string Code = "#include \"a.h\"\n"
+ "#include <vector>\n";
+ // FIXME: this might not be the best behavior.
+ std::string Expected = "#include \"a.h\"\n"
+ "#include \"vector\"\n"
+ "#include <vector>\n"
+ "#include <a.h>\n";
+ tooling::Replacements Replaces = {createInsertion("#include \"vector\""),
+ createInsertion("#include <a.h>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1486,7 +1486,6 @@
// code.
// FIXME: do not insert headers into conditional #include blocks, e.g. #includes
// surrounded by compile condition "#if...".
-// FIXME: do not insert existing headers.
// FIXME: insert empty lines between newly created blocks.
tooling::Replacements
fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
@@ -1533,10 +1532,12 @@
TrimmedCode.split(Lines, '\n');
unsigned Offset = MinInsertOffset;
unsigned NextLineOffset;
+ std::set<StringRef> ExistingIncludes;
for (auto Line : Lines) {
NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
if (IncludeRegex.match(Line, &Matches)) {
StringRef IncludeName = Matches[2];
+ ExistingIncludes.insert(IncludeName);
int Category = Categories.getIncludePriority(
IncludeName, /*CheckMainHeader=*/FirstIncludeOffset < 0);
CategoryEndOffsets[Category] = NextLineOffset;
@@ -1572,6 +1573,11 @@
"'#include ...'");
(void)Matched;
auto IncludeName = Matches[2];
+ if (ExistingIncludes.find(IncludeName) != ExistingIncludes.end()) {
+ DEBUG(llvm::dbgs() << "Skip adding existing include : " << IncludeName
+ << "\n");
+ continue;
+ }
int Category =
Categories.getIncludePriority(IncludeName, /*CheckMainHeader=*/true);
Offset = CategoryEndOffsets[Category];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
