Merged to 6.0 in r324331.
On Mon, Feb 5, 2018 at 4:59 PM, Mark Zeren via cfe-commits <cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits