[cfe-commits] r172819 - in /cfe/trunk: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp

[Nico Weber]

On Fri, Jan 18, 2013 at 6:04 AM, Manuel Klimek <klimek at google.com> wrote:
> Author: klimek
> Date: Fri Jan 18 08:04:34 2013
> New Revision: 172819
>
> URL: http://llvm.org/viewvc/llvm-project?rev=172819&view=rev
> Log:
> Fixes problems with line merging in the face of preprocessor directives.
>
> This patch prepares being able to test for and fix more problems (see
> FIXME in the test for example).
>
> Previously we would output unwrapped lines for preprocessor directives
> at the point where we also parsed the hash token. Since often
> projections only terminate (and thus output their own unwrapped line)
> after peeking at the next token, this would lead to the formatter seeing
> the preprocessor directives out-of-order (slightly earlier). To be able
> to correctly identify lines to merge, the formatter needs a well-defined
> order of unwrapped lines, which this patch introduces.
>
> 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=172819&r1=172818&r2=172819&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Fri Jan 18 08:04:34 2013
> @@ -79,7 +79,11 @@
>
>  class ScopedLineState {
>  public:
> -  ScopedLineState(UnwrappedLineParser &Parser) : Parser(Parser) {
> +  ScopedLineState(UnwrappedLineParser &Parser,
> +                  bool SwitchToPreprocessorLines = false)
> +      : Parser(Parser), SwitchToPreprocessorLines(SwitchToPreprocessorLines) {
> +    if (SwitchToPreprocessorLines)
> +      Parser.CurrentLines = &Parser.PreprocessorDirectives;
>      PreBlockLine = Parser.Line.take();
>      Parser.Line.reset(new UnwrappedLine());
>      Parser.Line->Level = PreBlockLine->Level;
> @@ -93,10 +97,13 @@
>      assert(Parser.Line->Tokens.empty());
>      Parser.Line.reset(PreBlockLine);
>      Parser.MustBreakBeforeNextToken = true;
> +    if (SwitchToPreprocessorLines)
> +      Parser.CurrentLines = &Parser.Lines;
>    }
>
>  private:
>    UnwrappedLineParser &Parser;
> +  const bool SwitchToPreprocessorLines;
>
>    UnwrappedLine *PreBlockLine;
>  };
> @@ -104,14 +111,20 @@
>  UnwrappedLineParser::UnwrappedLineParser(
>      clang::DiagnosticsEngine &Diag, const FormatStyle &Style,
>      FormatTokenSource &Tokens, UnwrappedLineConsumer &Callback)
> -    : Line(new UnwrappedLine), MustBreakBeforeNextToken(false), Diag(Diag),
> -      Style(Style), Tokens(&Tokens), Callback(Callback) {
> -}
> +    : Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
> +      CurrentLines(&Lines), Diag(Diag), Style(Style), Tokens(&Tokens),
> +      Callback(Callback) {}
>
>  bool UnwrappedLineParser::parse() {
>    DEBUG(llvm::dbgs() << "----\n");
>    readToken();
> -  return parseFile();
> +  bool Error = parseFile();
> +  for (std::vector<UnwrappedLine>::iterator I = Lines.begin(),
> +                                            E = Lines.end();
> +       I != E; ++I) {
> +    Callback.consumeUnwrappedLine(*I);
> +  }
> +  return Error;
>  }
>
>  bool UnwrappedLineParser::parseFile() {
> @@ -668,8 +681,17 @@
>      }
>      llvm::dbgs() << "\n";
>    });
> -  Callback.consumeUnwrappedLine(*Line);
> +  CurrentLines->push_back(*Line);
>    Line->Tokens.clear();
> +  if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
> +    for (std::vector<UnwrappedLine>::iterator I = PreprocessorDirectives
> +             .begin(), E = PreprocessorDirectives.end();
> +         I != E; ++I) {
> +      CurrentLines->push_back(*I);
> +    }
> +    PreprocessorDirectives.clear();
> +  }
> +
>  }
>
>  bool UnwrappedLineParser::eof() const {
> @@ -692,7 +714,11 @@
>    while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
>           ((FormatTok.NewlinesBefore > 0 && FormatTok.HasUnescapedNewline) ||
>            FormatTok.IsFirst)) {
> -    ScopedLineState BlockState(*this);
> +    // If there is an unfinished unwrapped line, we flush the preprocessor
> +    // directives only after that unwrapped line was finished later.
> +    bool SwitchToPreprocessorLines = !Line->Tokens.empty() &&
> +                                     CurrentLines == &Lines;
> +    ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
>      parsePPDirective();
>    }
>  }
>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=172819&r1=172818&r2=172819&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Fri Jan 18 08:04:34 2013
> @@ -159,6 +159,21 @@
>    FormatToken FormatTok;
>    bool MustBreakBeforeNextToken;
>
> +  // The parsed lines. This is a pointer

Doesn't look like a pointer.

> so we can switch it out to parse an
> +  // unrelated set of unwrapped lines and put them into place later.
> +  std::vector<UnwrappedLine> Lines;
> +
> +  // Preprocessor directives are parsed out-of-order from other unwrapped lines.
> +  // Thus, we need to keep a list of preprocessor directives to be reported
> +  // after an unwarpped line that has been started was finished.
> +  std::vector<UnwrappedLine> PreprocessorDirectives;
> +
> +  // New unwrapped lines are added via CurrentLines.
> +  // Usually points to \c &Lines. While parsing a preprocessor directive when
> +  // there is an unfinished previous unwrapped line, will point to
> +  // \c &PreprocessorDirectives.
> +  std::vector<UnwrappedLine> *CurrentLines;
> +
>    clang::DiagnosticsEngine &Diag;
>    const FormatStyle &Style;
>    FormatTokenSource *Tokens;
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172819&r1=172818&r2=172819&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Jan 18 08:04:34 2013
> @@ -1537,6 +1537,16 @@
>    EXPECT_EQ("#warning 1", format("  #  warning 1"));
>  }
>
> +TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
> +  FormatStyle AllowsMergedIf = getGoogleStyle();
> +  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
> +  verifyFormat("void f() { f(); }\n#error E", AllowsMergedIf);
> +  verifyFormat("if (true) return 42;\n#error E", AllowsMergedIf);
> +
> +  // FIXME:
> +  // verifyFormat("if (true)\n#error E\n  return 42;", AllowsMergedIf);
> +}
> +
>  // FIXME: This breaks the order of the unwrapped lines:
>  // TEST_F(FormatTest, OrderUnwrappedLines) {
>  //   verifyFormat("{\n"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits