r178261 - Fixed handling of comments before preprocessor directives.

[Daniel Jasper]

This causes some weirdness, e.g.:

namespace {
}
    // Return success.
#define A

Please fix.


On Tue, Apr 2, 2013 at 12:16 AM, Alexander Kornienko <alexfh at google.com>wrote:

> BTW, should I have looked into the date of the message before selecting
> the appropriate semantic parser and and reply generator? ;)
> On Apr 2, 2013 12:04 AM, "Alexander Kornienko" <alexfh at google.com> wrote:
>
>>
>> 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
>> >
>> >
>>
>
> _______________________________________________
> 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/626679eb/attachment.html>