Commit a6dc176b authored by Hans Wennborg's avatar Hans Wennborg
Browse files

Merging r324246: 

------------------------------------------------------------------------
r324246 | mzeren-vmw | 2018-02-05 16:59:00 +0100 (Mon, 05 Feb 2018) | 33 lines

[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
------------------------------------------------------------------------

llvm-svn: 324331
parent 0b67f617

clang/lib/Format/UnwrappedLineParser.cpp

+37 −23
Original line number Diff line number Diff line
@@ -234,14 +234,17 @@ UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style, 
      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();
@@ -712,26 +723,27 @@ void UnwrappedLineParser::parsePPIf(bool IfDef) { 
  // 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;
@@ -745,34 +757,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() ==
@@ -799,7 +814,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

clang/lib/Format/UnwrappedLineParser.h

+17 −4
Original line number Diff line number Diff line
@@ -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.

clang/unittests/Format/FormatTest.cpp

+14 −0
Original line number Diff line number Diff line
@@ -2532,6 +2532,20 @@ TEST_F(FormatTest, IndentPreprocessorDirectives) { 
               "#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