r324246 - [clang-format] Re-land: Fixup #include guard indents after parseFile()
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 6 01:55:27 PST 2018
Merged to 6.0 in r324331.
On Mon, Feb 5, 2018 at 4:59 PM, Mark Zeren via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list