Fixed in r171400 <http://llvm-reviews.chandlerc.com/rL171400>.
On Wed, Jan 2, 2013 at 6:11 PM, Daniel Jasper <[email protected]> wrote: > > > > On Wed, Jan 2, 2013 at 5:30 PM, Manuel Klimek <[email protected]> wrote: > >> Author: klimek >> Date: Wed Jan 2 10:30:12 2013 >> New Revision: 171393 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=171393&view=rev >> Log: >> Fixes use of unescaped newlines when formatting preprocessor directives. >> >> This is the first step towards handling preprocessor directives. This >> patch only fixes the most pressing issue, namely correctly escaping >> newlines for tokens within a sequence of a preprocessor directive. >> >> The next step will be to fix incorrect format decisions on #define >> directives. >> >> Modified: >> cfe/trunk/lib/Format/Format.cpp >> cfe/trunk/lib/Format/UnwrappedLineParser.cpp >> cfe/trunk/lib/Format/UnwrappedLineParser.h >> cfe/trunk/unittests/Format/FormatTest.cpp >> >> Modified: cfe/trunk/lib/Format/Format.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=171393&r1=171392&r2=171393&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Format/Format.cpp (original) >> +++ cfe/trunk/lib/Format/Format.cpp Wed Jan 2 10:30:12 2013 >> @@ -434,9 +434,21 @@ >> /// each \c FormatToken. >> void replaceWhitespace(const FormatToken &Tok, unsigned NewLines, >> unsigned Spaces) { >> + std::string NewLineText; >> + if (!Line.InPPDirective) { >> + NewLineText = std::string(NewLines, '\n'); >> + } else if (NewLines > 0) { >> + unsigned Offset = >> + SourceMgr.getSpellingColumnNumber(Tok.WhiteSpaceStart) - 1; >> > > This is probably wrong as we might have already changed the position of > the previous token. Instead, you need to pass in the column number of the > previous token. This is called from addTokenToState(). I would store the > previous column number (State.Column) there (immediately after "if > (Newline)") and pass it to replaceWhitespace(). > > >> + for (unsigned i = 0; i < NewLines; ++i) { >> + NewLineText += std::string(Style.ColumnLimit - Offset - 1, ' '); >> + NewLineText += "\\\n"; >> + Offset = 0; >> + } >> + } >> Replaces.insert(tooling::Replacement( >> SourceMgr, Tok.WhiteSpaceStart, Tok.WhiteSpaceLength, >> - std::string(NewLines, '\n') + std::string(Spaces, ' '))); >> + NewLineText + std::string(Spaces, ' '))); >> } >> >> /// \brief Add a new line and the required indent before the first >> Token >> @@ -634,6 +646,9 @@ >> next(); >> if (Index >= Tokens.size()) >> return; >> + // It is the responsibility of the UnwrappedLineParser to make sure >> + // this sequence is not produced inside an unwrapped line. >> + assert(Tokens[Index].Tok.getIdentifierInfo() != NULL); >> switch (Tokens[Index].Tok.getIdentifierInfo()->getPPKeywordID()) { >> case tok::pp_include: >> case tok::pp_import: >> @@ -969,7 +984,10 @@ >> >> // Consume and record whitespace until we find a significant token. >> while (FormatTok.Tok.is(tok::unknown)) { >> - FormatTok.NewlinesBefore += tokenText(FormatTok.Tok).count('\n'); >> + StringRef Text = tokenText(FormatTok.Tok); >> + FormatTok.NewlinesBefore += Text.count('\n'); >> + FormatTok.HasUnescapedNewline = >> + Text.count("\\\n") != FormatTok.NewlinesBefore; >> FormatTok.WhiteSpaceLength += FormatTok.Tok.getLength(); >> >> if (FormatTok.Tok.is(tok::eof)) >> >> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=171393&r1=171392&r2=171393&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original) >> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Jan 2 10:30:12 2013 >> @@ -80,13 +80,25 @@ >> } >> >> void UnwrappedLineParser::parsePPDirective() { >> - while (!eof()) { >> - nextToken(); >> - if (FormatTok.NewlinesBefore > 0) { >> - addUnwrappedLine(); >> - return; >> - } >> + assert(FormatTok.Tok.is(tok::hash) && "'#' expected"); >> + nextToken(); >> + >> + Line.InPPDirective = true; >> + if (FormatTok.Tok.getIdentifierInfo() == NULL) { >> + addUnwrappedLine(); >> + Line.InPPDirective = false; >> + return; >> } >> + >> + do { >> + if (FormatTok.NewlinesBefore > 0 && >> + FormatTok.HasUnescapedNewline) { >> + break; >> + } >> + nextToken(); >> + } while (!eof()); >> + addUnwrappedLine(); >> + Line.InPPDirective = false; >> } >> >> void UnwrappedLineParser::parseComments() { >> >> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=171393&r1=171392&r2=171393&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original) >> +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Wed Jan 2 10:30:12 2013 >> @@ -30,7 +30,8 @@ >> /// \brief A wrapper around a \c Token storing information about the >> /// whitespace characters preceeding it. >> struct FormatToken { >> - FormatToken() : NewlinesBefore(0), WhiteSpaceLength(0) { >> + FormatToken() >> + : NewlinesBefore(0), HasUnescapedNewline(false), >> WhiteSpaceLength(0) { >> } >> >> /// \brief The \c Token. >> @@ -42,6 +43,10 @@ >> /// and thereby e.g. leave an empty line between two function >> definitions. >> unsigned NewlinesBefore; >> >> + /// \brief Whether there is at least one unescaped newline before the >> \c >> + /// Token. >> + bool HasUnescapedNewline; >> + >> /// \brief The location of the start of the whitespace immediately >> preceeding >> /// the \c Token. >> /// >> @@ -60,7 +65,7 @@ >> /// \c UnwrappedLineFormatter. The key property is that changing the >> formatting >> /// within an unwrapped line does not affect any other unwrapped lines. >> struct UnwrappedLine { >> - UnwrappedLine() : Level(0) { >> + UnwrappedLine() : Level(0), InPPDirective(false) { >> } >> >> /// \brief The \c Token comprising this \c UnwrappedLine. >> @@ -68,6 +73,9 @@ >> >> /// \brief The indent level of the \c UnwrappedLine. >> unsigned Level; >> + >> + /// \brief Whether this \c UnwrappedLine is part of a preprocessor >> directive. >> + bool InPPDirective; >> }; >> >> class UnwrappedLineConsumer { >> >> Modified: cfe/trunk/unittests/Format/FormatTest.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=171393&r1=171392&r2=171393&view=diff >> >> ============================================================================== >> --- cfe/trunk/unittests/Format/FormatTest.cpp (original) >> +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 2 10:30:12 2013 >> @@ -373,6 +373,37 @@ >> " >> \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" };"); >> } >> >> +TEST_F(FormatTest, FormatsSmallMacroDefinitionsInSingleLine) { >> + verifyFormat("#define >> ALooooooooooooooooooooooooooooooooooooooongMacro(" >> + " \\\n" >> + " >> aLoooooooooooooooooooooooongFuuuuuuuuuuuuuunctiooooooooo)"); >> +} >> + >> +TEST_F(FormatTest, BreaksOnHashWhenDirectiveIsInvalid) { >> + EXPECT_EQ("#\n;", format("#;")); >> +} >> + >> +TEST_F(FormatTest, UnescapedEndOfLineEndsPPDirective) { >> + EXPECT_EQ("#line 42 \"test\"\n", >> + format("# \\\n line \\\n 42 \\\n \"test\"\n")); >> + EXPECT_EQ("#define A B\n", format("# \\\n define \\\n A \\\n >> B\n")); >> +} >> + >> +TEST_F(FormatTest, EndOfFileEndsPPDirective) { >> + EXPECT_EQ("#line 42 \"test\"", >> + format("# \\\n line \\\n 42 \\\n \"test\"")); >> + EXPECT_EQ("#define A B", format("# \\\n define \\\n A \\\n >> B")); >> +} >> + >> +TEST_F(FormatTest, MixingPreprocessorDirectivesAndNormalCode) { >> + verifyFormat("#define >> ALooooooooooooooooooooooooooooooooooooooongMacro(" >> + " \\\n" >> + " >> aLoooooooooooooooooooooooongFuuuuuuuuuuuuuunctiooooooooo)\n" >> + "\n" >> + >> "AlooooooooooooooooooooooooooooooooooooooongCaaaaaaaaaal(\n" >> + " >> aLooooooooooooooooooooooonPaaaaaaaaaaaaaaaaaaaaarmmmm);\n"); >> +} >> + >> >> >> //===----------------------------------------------------------------------===// >> // Line break tests. >> >> >> //===----------------------------------------------------------------------===// >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
