r195256 - Added an option to allow short function bodies be placed on a single line.

Manuel Klimek klimek at google.com
Thu Nov 21 07:11:23 PST 2013


So, in general, I think the problem is that we still need to break in the
"fits into one line" case in some styles:
void f () {
  f();
}
Here if we have a style that requires the { to go on the next line, we'd
still generate at least 3 unwrapped lines; thus, even if "void f() {" fits
into one line, we need to put the break before the { in somewhere, unless
all of "void f() { f(); }" fits into one line and we can join the lines...

Where would you propose to do that?


On Thu, Nov 21, 2013 at 4:04 PM, Manuel Klimek <klimek at google.com> wrote:

> On Thu, Nov 21, 2013 at 3:56 PM, Daniel Jasper <djasper at google.com> wrote:
>
>> I don't feel strongly about this. However, putting then decision into
>> mustBreak would just work. You don't need any special casing for the
>> single-line case as that is handled by a different code path..
>>
>
> I don't understand that yet - if mustBreak retruns true, we'd introduce a
> break; when we join lines, we'd need to remove that break again, and count
> the right number of spaces, right?
>
>
>>  On Nov 21, 2013 2:22 AM, "Alexander Kornienko" <alexfh at google.com>
>> wrote:
>>
>>> Having played with this a bit, I found a few problems with not putting
>>> the braces into separate lines in the UnwrappedLinesParser:
>>>   * if we have braces on the same unwrapped line, we'll need to
>>> introduce a break when laying them out (using TokenAnnotator::mustBreak),
>>> and we'll have to undo this break when joining lines (IIUC, line joiner
>>> currently doesn't support this);
>>>   * when MustBreakBefore is set, we also make TotalLength > ColumnLimit,
>>> and we'll need to undo this in line joiner, which will also add complexity.
>>>
>>> Overall, always having the braces on the same unwrapped line doesn't
>>> seem to be able to simplify the code =\
>>>
>>>
>>> On Wed, Nov 20, 2013 at 7:32 PM, Daniel Jasper <djasper at google.com>wrote:
>>>
>>>>
>>>> On Wed, Nov 20, 2013 at 9:28 AM, Alexander Kornienko <alexfh at google.com
>>>> > wrote:
>>>>
>>>>> On Wed, Nov 20, 2013 at 6:12 PM, Daniel Jasper <djasper at google.com>wrote:
>>>>>
>>>>>>
>>>>>>> @@ -681,6 +681,7 @@ void UnwrappedLineParser::parseStructura
>>>>>>>              Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup ||
>>>>>>>              Style.BreakBeforeBraces == FormatStyle::BS_Allman)
>>>>>>>            addUnwrappedLine();
>>>>>>>
>>>>>>
>>>>>> Does it still make sense to report the "{" as its own unwrapped line?
>>>>>> Seems a bit convoluted to first report multiple lines and then merge them
>>>>>> afterwards. I think this would make the merging code simpler.
>>>>>>
>>>>>
>>>>> It also seemed strange to me. Should we instead handle
>>>>> BreakBeforeBraces in TokenAnnotator? This will require adding TokenType
>>>>> values for braces starting namespaces, classes/structs and, probably,
>>>>> enums. I can play with this a bit, it you think it makes sense.
>>>>>
>>>>
>>>> I might have already done this for enums. I don't think it is essential
>>>> to add token types for all of these as e.g. enums and namespaces are really
>>>> easy to detect. But adding token types might be the cleaner solution. I
>>>> think that this makes sense but I remember having some kind of debate over
>>>> this with Manuel, so he might have an opinion.
>>>>
>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131121/6b8031b7/attachment.html>


More information about the cfe-commits mailing list