This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix AlwaysBreakAfterDefinitionReturnType incompatibility with BreakBeforeBraces: Stroustrup
ClosedPublic

Authored by curdeius on Aug 13 2014, 9:35 AM.

Details

Reviewers
djasper
Summary

I have found a bug when using a style like:

AlwaysBreakAfterDefinitionReturnType: true
BreakBeforeBraces: Stroustrup

I attach a straightforward patch that passes the tests.
However, I think that the issue may be actually somewhere deeper, e.g. in Line.Last pointing incorrectly to a different token than l_brace (cf. the patch).

Diff Detail

Event Timeline

curdeius updated this revision to Diff 12459.Aug 13 2014, 9:35 AM
curdeius retitled this revision from to [clang-format] Fix AlwaysBreakAfterDefinitionReturnType incompatibility with BreakBeforeBraces: Stroustrup.
curdeius updated this object.
curdeius edited the test plan for this revision. (Show Details)
curdeius added a reviewer: djasper.
curdeius set the repository for this revision to rL LLVM.
curdeius added a project: deleted.
curdeius added a subscriber: curdeius.
curdeius added a subscriber: Unknown Object (MLST).Aug 13 2014, 9:37 AM
curdeius set the repository for this revision to rL LLVM.Aug 14 2014, 1:14 AM
djasper accepted this revision.Aug 14 2014, 1:28 AM
djasper edited edge metadata.

I am happy to get this in as is especially as it is good to have the tests. I also agree that there is an underlying issue which we should look at in more detail. Can you add a FIXME to this effect?

lib/Format/TokenAnnotator.cpp
1293

Can you make this:

!Line.Last->isOneOf(tok::semi, tok::comment)

That seems to be the most obvious exception I can think of.

This revision is now accepted and ready to land.Aug 14 2014, 1:28 AM
curdeius updated this revision to Diff 12491.Aug 14 2014, 2:35 AM
curdeius updated this object.
curdeius edited the test plan for this revision. (Show Details)
curdeius edited edge metadata.
  • [clang-format] Fix {AlwaysBreakAfterDefinitionReturnType: true} not working with {BreakBeforeBraces: Stroustrup}.
  • [clang-format] Add FIXME and check for a tok::semi or a tok::comment.
curdeius updated this revision to Diff 12493.Aug 14 2014, 2:40 AM
  • Format.

Do you have commit access or should I commit this for you?

I don't have it. Please commit it.

djasper closed this revision.Aug 14 2014, 4:45 AM

Submitted as r215633.