[cfe-commits] r172819 - in /cfe/trunk: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp
Manuel Klimek
klimek at google.com
Fri Jan 18 10:25:15 PST 2013
On Fri, Jan 18, 2013 at 5:07 PM, Nico Weber <thakis at chromium.org> wrote:
> 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.
>
Design changed, comment didnt :) Fixed in r172831. Thx for spotting.
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130118/4b55f004/attachment.html>
More information about the cfe-commits
mailing list