Page MenuHomePhabricator

[clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change
ClosedPublic

Authored by MyDeveloperDay on Aug 27 2021, 1:56 AM.

Details

Summary

LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping (likely before we simply didn't wrap) but may be related to D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

Logged as https://bugs.llvm.org/show_bug.cgi?id=51640

This change ensure AfterEnum works for

internal|public|protected|private enum A { in the same way as it works for enum A { in C++

A similar issue was also observed with interface in C#

Diff Detail

Event Timeline

MyDeveloperDay requested review of this revision.Aug 27 2021, 1:56 AM
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: owenpan, exv, jbcoe.

Remove the unnecessary comment changes

krasimir accepted this revision.Aug 27 2021, 2:22 AM
krasimir added inline comments.
clang/lib/Format/TokenAnnotator.cpp
3818

nit: the && Style.BraceWrapping.AfterClass is unnecessary since we've ensured that in the parent if. I'd just fold the kw_interface case into the previous if, and then it seems we can further fold the remaining inner if into the parent if, resulting in this whole section (lines 3796--3805) turning into a single large if statement.

This revision is now accepted and ready to land.Aug 27 2021, 2:22 AM
MyDeveloperDay marked an inline comment as done.

Address Nit to reduce if nesting

[----------] Global test environment tear-down
[==========] 814 tests from 25 test suites ran. (41295 ms total)
[  PASSED  ] 814 tests.
clang/lib/Format/TokenAnnotator.cpp
3818

nice catch, will do

owenpan added inline comments.Aug 27 2021, 4:38 AM
clang/lib/Format/TokenAnnotator.cpp
3803–3805

Nit: can you add a boolean above the outer if so that it can be used in the if below?

Also, Line.First is inconsistent with Line.startsWith elsewhere? I think the latter skips leading comments.

Handle comments before the enum

MyDeveloperDay marked an inline comment as done.Aug 27 2021, 6:03 AM
krasimir accepted this revision.Aug 27 2021, 6:06 AM

LGTM.

clang/lib/Format/TokenAnnotator.cpp
3804

Nit: I think it would be a little more efficient to call FirstNonComment->is instead.

owenpan accepted this revision.Aug 27 2021, 6:20 AM
This revision was landed with ongoing or failed builds.Aug 27 2021, 11:14 AM
This revision was automatically updated to reflect the committed changes.