[cfe-commits] r171393 - in /cfe/trunk: lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp
Manuel Klimek
klimek at google.com
Wed Jan 2 10:59:43 PST 2013
Fixed in r171400 <http://llvm-reviews.chandlerc.com/rL171400>.
On Wed, Jan 2, 2013 at 6:11 PM, Daniel Jasper <djasper at google.com> wrote:
>
>
>
> On Wed, Jan 2, 2013 at 5:30 PM, Manuel Klimek <klimek at google.com> 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
>> 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/20130102/611fce28/attachment.html>
More information about the cfe-commits
mailing list