This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] New flag - BraceWrapping.AfterExternBlock
ClosedPublic

Authored by PriMee on Sep 14 2017, 1:09 AM.

Details

Summary

Bug: https://bugs.llvm.org/show_bug.cgi?id=34016 - "extern C part"

Problem:

Due to the lack of "brace wrapping extern" flag, clang format does parse the block after extern keyword moving the opening bracket to the header line always!

Patch description:

A new style added, new configuration flag - BraceWrapping.AfterExternBlock that allows us to decide whether we want a break before brace or not.

Diff Detail

Event Timeline

PriMee created this revision.Sep 14 2017, 1:09 AM
krasimir added inline comments.Sep 14 2017, 2:47 AM
docs/ClangFormatStyleOptions.rst
664

I think this is overly specific. The C++ standard also [[ http://en.cppreference.com/w/cpp/language/language_linkage | allows extern "C++" blocks ]]. I'd rename this to AfterExternBlock or something similar.

lib/Format/UnwrappedLineParser.cpp
1044

I think we should keep the old /*AddLevel=*/false when AfterExternC is false. That way you also don't have to change the following test case.

PriMee edited the summary of this revision. (Show Details)Sep 14 2017, 3:13 AM
PriMee retitled this revision from [clang-format] New flag - BraceWrapping.AfterExternC to [clang-format] New flag - BraceWrapping.AfterExternBlock.Sep 14 2017, 3:38 AM
krasimir added inline comments.Sep 14 2017, 5:03 AM
lib/Format/UnwrappedLineParser.cpp
1047

Please clang-format the newly added lines.

PriMee updated this revision to Diff 115202.Sep 14 2017, 5:35 AM

Sorry, forgot again...

krasimir edited edge metadata.Sep 14 2017, 6:31 AM

Looks great! Just one more nit: please add a line checking the parsing of the new option similar to the line CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct); in unittests/Format/FormatTest.cpp.

PriMee updated this revision to Diff 115240.Sep 14 2017, 10:20 AM

Thank you for noticing! Done.

krasimir accepted this revision.Sep 15 2017, 1:52 AM

Looks good! Should I commit this for you?

This revision is now accepted and ready to land.Sep 15 2017, 1:52 AM

I would be grateful, thank you!

This revision was automatically updated to reflect the committed changes.