r271883 - [clang-format] make header guard identification stricter (with Lexer).
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 6 04:00:14 PDT 2016
Author: ioeric
Date: Mon Jun 6 06:00:13 2016
New Revision: 271883
URL: http://llvm.org/viewvc/llvm-project?rev=271883&view=rev
Log:
[clang-format] make header guard identification stricter (with Lexer).
Summary: make header guard identification stricter with Lexer.
Reviewers: djasper
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D20959
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/CleanupTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=271883&r1=271882&r2=271883&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Jun 6 06:00:13 2016
@@ -1436,6 +1436,49 @@ inline bool isHeaderInsertion(const tool
llvm::Regex(IncludeRegexPattern).match(Replace.getReplacementText());
}
+void skipComments(Lexer &Lex, Token &Tok) {
+ while (Tok.is(tok::comment))
+ if (Lex.LexFromRawLexer(Tok))
+ return;
+}
+
+// Check if a sequence of tokens is like "#<Name> <raw_identifier>". If it is,
+// \p Tok will be the token after this directive; otherwise, it can be any token
+// after the given \p Tok (including \p Tok).
+bool checkAndConsumeDirectiveWithName(Lexer &Lex, StringRef Name, Token &Tok) {
+ bool Matched = Tok.is(tok::hash) && !Lex.LexFromRawLexer(Tok) &&
+ Tok.is(tok::raw_identifier) &&
+ Tok.getRawIdentifier() == Name && !Lex.LexFromRawLexer(Tok) &&
+ Tok.is(tok::raw_identifier);
+ if (Matched)
+ Lex.LexFromRawLexer(Tok);
+ return Matched;
+}
+
+unsigned getOffsetAfterHeaderGuardsAndComments(StringRef FileName,
+ StringRef Code,
+ FormatStyle Style) {
+ std::unique_ptr<Environment> Env =
+ Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+ const SourceManager &SourceMgr = Env->getSourceManager();
+ Lexer Lex(Env->getFileID(), SourceMgr.getBuffer(Env->getFileID()), SourceMgr,
+ getFormattingLangOpts(Style));
+ Token Tok;
+ // Get the first token.
+ Lex.LexFromRawLexer(Tok);
+ skipComments(Lex, Tok);
+ unsigned AfterComments = SourceMgr.getFileOffset(Tok.getLocation());
+ if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
+ skipComments(Lex, Tok);
+ if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+ return SourceMgr.getFileOffset(Tok.getLocation());
+ }
+ return AfterComments;
+}
+
+// FIXME: we also need to insert a '\n' at the end of the code if we have an
+// insertion with offset Code.size(), and there is no '\n' at the end of the
+// code.
// FIXME: do not insert headers into conditional #include blocks, e.g. #includes
// surrounded by compile condition "#if...".
// FIXME: do not insert existing headers.
@@ -1469,20 +1512,6 @@ fixCppIncludeInsertions(StringRef Code,
StringRef FileName = Replaces.begin()->getFilePath();
IncludeCategoryManager Categories(Style, FileName);
- std::unique_ptr<Environment> Env =
- Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
- const SourceManager &SourceMgr = Env->getSourceManager();
- Lexer Lex(Env->getFileID(), SourceMgr.getBuffer(Env->getFileID()), SourceMgr,
- getFormattingLangOpts(Style));
- Token Tok;
- // All new headers should be inserted after this offset.
- int MinInsertOffset = Code.size();
- while (!Lex.LexFromRawLexer(Tok)) {
- if (Tok.isNot(tok::comment)) {
- MinInsertOffset = SourceMgr.getFileOffset(Tok.getLocation());
- break;
- }
- }
// Record the offset of the end of the last include in each category.
std::map<int, int> CategoryEndOffsets;
// All possible priorities.
@@ -1491,26 +1520,25 @@ fixCppIncludeInsertions(StringRef Code,
for (const auto &Category : Style.IncludeCategories)
Priorities.insert(Category.Priority);
int FirstIncludeOffset = -1;
- bool HeaderGuardFound = false;
+ // All new headers should be inserted after this offset.
+ unsigned MinInsertOffset =
+ getOffsetAfterHeaderGuardsAndComments(FileName, Code, Style);
StringRef TrimmedCode = Code.drop_front(MinInsertOffset);
SmallVector<StringRef, 32> Lines;
TrimmedCode.split(Lines, '\n');
- int Offset = MinInsertOffset;
+ unsigned Offset = MinInsertOffset;
+ unsigned NextLineOffset;
for (auto Line : Lines) {
+ NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
if (IncludeRegex.match(Line, &Matches)) {
StringRef IncludeName = Matches[2];
int Category = Categories.getIncludePriority(
IncludeName, /*CheckMainHeader=*/FirstIncludeOffset < 0);
- CategoryEndOffsets[Category] = Offset + Line.size() + 1;
+ CategoryEndOffsets[Category] = NextLineOffset;
if (FirstIncludeOffset < 0)
FirstIncludeOffset = Offset;
}
- Offset += Line.size() + 1;
- // FIXME: make header guard matching stricter, e.g. consider #ifndef.
- if (!HeaderGuardFound && DefineRegex.match(Line)) {
- HeaderGuardFound = true;
- MinInsertOffset = Offset;
- }
+ Offset = NextLineOffset;
}
// Populate CategoryEndOfssets:
Modified: cfe/trunk/unittests/Format/CleanupTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/CleanupTest.cpp?rev=271883&r1=271882&r2=271883&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/CleanupTest.cpp (original)
+++ cfe/trunk/unittests/Format/CleanupTest.cpp Mon Jun 6 06:00:13 2016
@@ -608,6 +608,86 @@ TEST_F(CleanUpReplacementsTest, CodeAfte
EXPECT_EQ(Expected, apply(Code, Replaces));
}
+TEST_F(CleanUpReplacementsTest, FakeHeaderGuardIfDef) {
+ std::string Code = "// comment \n"
+ "#ifdef X\n"
+ "#define X\n";
+ std::string Expected = "// comment \n"
+ "#include <vector>\n"
+ "#ifdef X\n"
+ "#define X\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, RealHeaderGuardAfterComments) {
+ std::string Code = "// comment \n"
+ "#ifndef X\n"
+ "#define X\n"
+ "int x;\n"
+ "#define Y 1\n";
+ std::string Expected = "// comment \n"
+ "#ifndef X\n"
+ "#define X\n"
+ "#include <vector>\n"
+ "int x;\n"
+ "#define Y 1\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, IfNDefWithNoDefine) {
+ std::string Code = "// comment \n"
+ "#ifndef X\n"
+ "int x;\n"
+ "#define Y 1\n";
+ std::string Expected = "// comment \n"
+ "#include <vector>\n"
+ "#ifndef X\n"
+ "int x;\n"
+ "#define Y 1\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, HeaderGuardWithComment) {
+ std::string Code = "// comment \n"
+ "#ifndef X // comment\n"
+ "// comment\n"
+ "/* comment\n"
+ "*/\n"
+ "/* comment */ #define X\n"
+ "int x;\n"
+ "#define Y 1\n";
+ std::string Expected = "// comment \n"
+ "#ifndef X // comment\n"
+ "// comment\n"
+ "/* comment\n"
+ "*/\n"
+ "/* comment */ #define X\n"
+ "#include <vector>\n"
+ "int x;\n"
+ "#define Y 1\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, EmptyCode) {
+ std::string Code = "";
+ std::string Expected = "#include <vector>\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+// FIXME: although this case does not crash, the insertion is wrong. A '\n'
+// should be inserted between the two #includes.
+TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCode) {
+ std::string Code = "#include <map>";
+ std::string Expected = "#include <map>#include <vector>\n";
+ tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+ EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
More information about the cfe-commits
mailing list