r324246 - [clang-format] Re-land: Fixup #include guard indents after parseFile()
Mark Zeren via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 5 07:59:00 PST 2018
Author: mzeren-vmw
Date: Mon Feb 5 07:59:00 2018
New Revision: 324246
URL: http://llvm.org/viewvc/llvm-project?rev=324246&view=rev
Log:
[clang-format] Re-land: Fixup #include guard indents after parseFile()
Summary:
When a preprocessor indent closes after the last line of normal code we do not
correctly fixup include guard indents. For example:
#ifndef HEADER_H
#define HEADER_H
#if 1
int i;
# define A 0
#endif
#endif
incorrectly reformats to:
#ifndef HEADER_H
#define HEADER_H
#if 1
int i;
# define A 0
# endif
#endif
To resolve this issue we must fixup levels after parseFile(). Delaying
the fixup introduces a new state, so consolidate include guard search
state into an enum.
Reviewers: krasimir, klimek
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D42035
Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=324246&r1=324245&r2=324246&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Feb 5 07:59:00 2018
@@ -234,14 +234,17 @@ UnwrappedLineParser::UnwrappedLineParser
CurrentLines(&Lines), Style(Style), Keywords(Keywords),
CommentPragmasRegex(Style.CommentPragmas), Tokens(nullptr),
Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1),
- IfNdefCondition(nullptr), FoundIncludeGuardStart(false),
- IncludeGuardRejected(false), FirstStartColumn(FirstStartColumn) {}
+ IncludeGuard(Style.IndentPPDirectives == FormatStyle::PPDIS_None
+ ? IG_Rejected
+ : IG_Inited),
+ IncludeGuardToken(nullptr), FirstStartColumn(FirstStartColumn) {}
void UnwrappedLineParser::reset() {
PPBranchLevel = -1;
- IfNdefCondition = nullptr;
- FoundIncludeGuardStart = false;
- IncludeGuardRejected = false;
+ IncludeGuard = Style.IndentPPDirectives == FormatStyle::PPDIS_None
+ ? IG_Rejected
+ : IG_Inited;
+ IncludeGuardToken = nullptr;
Line.reset(new UnwrappedLine);
CommentsBeforeNextToken.clear();
FormatTok = nullptr;
@@ -264,6 +267,14 @@ void UnwrappedLineParser::parse() {
readToken();
parseFile();
+
+ // If we found an include guard then all preprocessor directives (other than
+ // the guard) are over-indented by one.
+ if (IncludeGuard == IG_Found)
+ for (auto &Line : Lines)
+ if (Line.InPPDirective && Line.Level > 0)
+ --Line.Level;
+
// Create line with eof token.
pushToken(FormatTok);
addUnwrappedLine();
@@ -724,26 +735,27 @@ void UnwrappedLineParser::parsePPIf(bool
// If there's a #ifndef on the first line, and the only lines before it are
// comments, it could be an include guard.
bool MaybeIncludeGuard = IfNDef;
- if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard) {
+ if (IncludeGuard == IG_Inited && MaybeIncludeGuard)
for (auto &Line : Lines) {
if (!Line.Tokens.front().Tok->is(tok::comment)) {
MaybeIncludeGuard = false;
- IncludeGuardRejected = true;
+ IncludeGuard = IG_Rejected;
break;
}
}
- }
--PPBranchLevel;
parsePPUnknown();
++PPBranchLevel;
- if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard)
- IfNdefCondition = IfCondition;
+ if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
+ IncludeGuard = IG_IfNdefed;
+ IncludeGuardToken = IfCondition;
+ }
}
void UnwrappedLineParser::parsePPElse() {
// If a potential include guard has an #else, it's not an include guard.
- if (FoundIncludeGuardStart && PPBranchLevel == 0)
- FoundIncludeGuardStart = false;
+ if (IncludeGuard == IG_Defined && PPBranchLevel == 0)
+ IncludeGuard = IG_Rejected;
conditionalCompilationAlternative();
if (PPBranchLevel > -1)
--PPBranchLevel;
@@ -757,34 +769,37 @@ void UnwrappedLineParser::parsePPEndIf()
conditionalCompilationEnd();
parsePPUnknown();
// If the #endif of a potential include guard is the last thing in the file,
- // then we count it as a real include guard and subtract one from every
- // preprocessor indent.
+ // then we found an include guard.
unsigned TokenPosition = Tokens->getPosition();
FormatToken *PeekNext = AllTokens[TokenPosition];
- if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof) &&
+ if (IncludeGuard == IG_Defined && PPBranchLevel == -1 &&
+ PeekNext->is(tok::eof) &&
Style.IndentPPDirectives != FormatStyle::PPDIS_None)
- for (auto &Line : Lines)
- if (Line.InPPDirective && Line.Level > 0)
- --Line.Level;
+ IncludeGuard = IG_Found;
}
void UnwrappedLineParser::parsePPDefine() {
nextToken();
if (FormatTok->Tok.getKind() != tok::identifier) {
+ IncludeGuard = IG_Rejected;
+ IncludeGuardToken = nullptr;
parsePPUnknown();
return;
}
- if (IfNdefCondition && IfNdefCondition->TokenText == FormatTok->TokenText) {
- FoundIncludeGuardStart = true;
+
+ if (IncludeGuard == IG_IfNdefed &&
+ IncludeGuardToken->TokenText == FormatTok->TokenText) {
+ IncludeGuard = IG_Defined;
+ IncludeGuardToken = nullptr;
for (auto &Line : Lines) {
if (!Line.Tokens.front().Tok->isOneOf(tok::comment, tok::hash)) {
- FoundIncludeGuardStart = false;
+ IncludeGuard = IG_Rejected;
break;
}
}
}
- IfNdefCondition = nullptr;
+
nextToken();
if (FormatTok->Tok.getKind() == tok::l_paren &&
FormatTok->WhitespaceRange.getBegin() ==
@@ -811,7 +826,6 @@ void UnwrappedLineParser::parsePPUnknown
if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
Line->Level += PPBranchLevel + 1;
addUnwrappedLine();
- IfNdefCondition = nullptr;
}
// Here we blacklist certain tokens that are not usually the first token in an
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=324246&r1=324245&r2=324246&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Feb 5 07:59:00 2018
@@ -248,10 +248,23 @@ private:
// sequence.
std::stack<int> PPChainBranchIndex;
- // Contains the #ifndef condition for a potential include guard.
- FormatToken *IfNdefCondition;
- bool FoundIncludeGuardStart;
- bool IncludeGuardRejected;
+ // Include guard search state. Used to fixup preprocessor indent levels
+ // so that include guards do not participate in indentation.
+ enum IncludeGuardState {
+ IG_Inited, // Search started, looking for #ifndef.
+ IG_IfNdefed, // #ifndef found, IncludeGuardToken points to condition.
+ IG_Defined, // Matching #define found, checking other requirements.
+ IG_Found, // All requirements met, need to fix indents.
+ IG_Rejected, // Search failed or never started.
+ };
+
+ // Current state of include guard search.
+ IncludeGuardState IncludeGuard;
+
+ // Points to the #ifndef condition for a potential include guard. Null unless
+ // IncludeGuardState == IG_IfNdefed.
+ FormatToken *IncludeGuardToken;
+
// Contains the first start column where the source begins. This is zero for
// normal source code and may be nonzero when formatting a code fragment that
// does not start at the beginning of the file.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=324246&r1=324245&r2=324246&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Feb 5 07:59:00 2018
@@ -2547,6 +2547,20 @@ TEST_F(FormatTest, IndentPreprocessorDir
"#elif FOO\n"
"#endif",
Style);
+ // Non-identifier #define after potential include guard.
+ verifyFormat("#ifndef FOO\n"
+ "# define 1\n"
+ "#endif\n",
+ Style);
+ // #if closes past last non-preprocessor line.
+ verifyFormat("#ifndef FOO\n"
+ "#define FOO\n"
+ "#if 1\n"
+ "int i;\n"
+ "# define A 0\n"
+ "#endif\n"
+ "#endif\n",
+ Style);
// FIXME: This doesn't handle the case where there's code between the
// #ifndef and #define but all other conditions hold. This is because when
// the #define line is parsed, UnwrappedLineParser::Lines doesn't hold the
More information about the cfe-commits
mailing list