r178261 - Fixed handling of comments before preprocessor directives.

[Alexander Kornienko]

On Apr 1, 2013 6:21 PM, "Manuel Klimek" <klimek at google.com> wrote:
>
> On Thu, Mar 28, 2013 at 7:40 PM, Alexander Kornienko <alexfh at google.com>
wrote:
>>
>> Author: alexfh
>> Date: Thu Mar 28 13:40:55 2013
>> New Revision: 178261
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=178261&view=rev
>> Log:
>> Fixed handling of comments before preprocessor directives.
>>
>> Comments before preprocessor directives used to be stored with
InPPDirective
>> flag set, which prevented correct comment splitting in this case. Fixed
by
>> flushing comments before switching on InPPDirective. Added a new test
and fixed
>> one of the existing tests.
>>
>>
>> Modified:
>>     cfe/trunk/lib/Format/UnwrappedLineParser.cpp
>>     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=178261&r1=178260&r2=178261&view=diff
>>
==============================================================================
>> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
>> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Thu Mar 28 13:40:55 2013
>> @@ -822,6 +822,7 @@ void UnwrappedLineParser::readToken() {
>>      while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
>>             ((FormatTok.NewlinesBefore > 0 &&
FormatTok.HasUnescapedNewline) ||
>>              FormatTok.IsFirst)) {
>
>
> Please add a comment indicating why we need this.

By "this" you mean the flushComments call I suppose? I wanted to add a
comment, but "flush comments before starting to parse the preprocessor
directive" seems like rephrasing the code. Or you can suggest a better
comment?

>
>>
>> +      flushComments(FormatTok.NewlinesBefore > 0);
>>        // If there is an unfinished unwrapped line, we flush the
preprocessor
>>        // directives only after that unwrapped line was finished later.
>>        bool SwitchToPreprocessorLines =
>>
>> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=178261&r1=178260&r2=178261&view=diff
>>
==============================================================================
>> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
>> +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Mar 28 13:40:55 2013
>> @@ -699,6 +699,13 @@ TEST_F(FormatTest, SplitsLongCxxComments
>>        "//Don't add leading\n"
>>        "//whitespace",
>>        format("//Don't add leading whitespace",
getLLVMStyleWithColumns(20)));
>> +  EXPECT_EQ("// A comment before\n"
>> +            "// a macro\n"
>> +            "// definition\n"
>> +            "#define a b",
>> +            format("// A comment before a macro definition\n"
>> +                   "#define a b",
>> +                   getLLVMStyleWithColumns(20)));
>>  }
>>
>>  TEST_F(FormatTest, SplitsLongLinesInComments) {
>> @@ -1203,13 +1210,13 @@ TEST_F(FormatTest, IndentsPPDirectiveInR
>>  }
>>
>>  TEST_F(FormatTest, HandlePreprocessorDirectiveContext) {
>> -  EXPECT_EQ("// some comment\n"
>> +  EXPECT_EQ("// somecomment\n"
>
>
> What's the reason for this part of the change?

This is the smallest change possible to make this test pass after we
started breaking long line comments. There are many alternative solutions,
of course.

>
>>
>>              "#include \"a.h\"\n"
>>              "#define A(  \\\n"
>>              "    A, B)\n"
>>              "#include \"b.h\"\n"
>>              "// somecomment\n",
>> -            format("  // some comment\n"
>> +            format("  // somecomment\n"
>>                     "  #include \"a.h\"\n"
>>                     "#define A(A,\\\n"
>>                     "    B)\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/20130402/33e30499/attachment.html>