Skip to content

Commit e743114

Browse files
HazardyKnusperkeksHazardyKnusperkeks
committed
[clang-format] Remove special handling of comments after brace/paren
Fixes http://llvm.org/PR55487 (#55487) The code did not match the documentation about Cpp11BracedListStyle. Changed handling of comments after opening braces, which are supposedly function call like to behave exactly like their parenthesis counter part.
1 parent 37af81f commit e743114

14 files changed

+257
-82
lines changed

‎clang/include/clang/Format/Format.h‎

Lines changed: 59 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2555,29 +2555,66 @@ struct FormatStyle {
25552555
/// \version 3.7
25562556
unsigned ContinuationIndentWidth;
25572557

2558-
/// If ``true``, format braced lists as best suited for C++11 braced
2559-
/// lists.
2560-
///
2561-
/// Important differences:
2562-
///
2563-
/// * No spaces inside the braced list.
2564-
/// * No line break before the closing brace.
2565-
/// * Indentation with the continuation indent, not with the block indent.
2566-
///
2567-
/// Fundamentally, C++11 braced lists are formatted exactly like function
2568-
/// calls would be formatted in their place. If the braced list follows a name
2569-
/// (e.g. a type or variable name), clang-format formats as if the ``{}`` were
2570-
/// the parentheses of a function call with that name. If there is no name,
2571-
/// a zero-length name is assumed.
2572-
/// \code
2573-
/// true: false:
2574-
/// vector<int> x{1, 2, 3, 4}; vs. vector<int> x{ 1, 2, 3, 4 };
2575-
/// vector<T> x{{}, {}, {}, {}}; vector<T> x{ {}, {}, {}, {} };
2576-
/// f(MyMap[{composite, key}]); f(MyMap[{ composite, key }]);
2577-
/// new int[3]{1, 2, 3}; new int[3]{ 1, 2, 3 };
2578-
/// \endcode
2558+
/// Different ways to handle braced lists.
2559+
enum CppBracedListStyleStyle : int8_t {
2560+
/// Best suited for pre C++11 braced lists.
2561+
///
2562+
/// * Spaces inside the braced list.
2563+
/// * Line break before the closing brace.
2564+
/// * Indentation with the block indent.
2565+
///
2566+
/// \code
2567+
/// vector<int> x{ 1, 2, 3, 4 };
2568+
/// vector<T> x{ {}, {}, {}, {} };
2569+
/// f(MyMap[{ composite, key }]);
2570+
/// new int[3]{ 1, 2, 3 };
2571+
/// Type name{ //Comment
2572+
/// value
2573+
/// };
2574+
/// \endcode
2575+
CBLSS_Block,
2576+
/// Best suited for C++11 braced lists.
2577+
///
2578+
/// * No spaces inside the braced list.
2579+
/// * No line break before the closing brace.
2580+
/// * Indentation with the continuation indent.
2581+
///
2582+
/// Fundamentally, C++11 braced lists are formatted exactly like function
2583+
/// calls would be formatted in their place. If the braced list follows a
2584+
/// name (e.g. a type or variable name), clang-format formats as if the
2585+
/// ``{}`` were the parentheses of a function call with that name. If there
2586+
/// is no name, a zero-length name is assumed.
2587+
/// \code
2588+
/// vector<int> x{1, 2, 3, 4};
2589+
/// vector<T> x{{}, {}, {}, {}};
2590+
/// f(MyMap[{composite, key}]);
2591+
/// new int[3]{1, 2, 3};
2592+
/// Type name{ //Comment
2593+
/// value};
2594+
/// \endcode
2595+
CBLSS_FunctionCall,
2596+
/// Same as ``FunctionCall``, except for the handling of a comment at the
2597+
/// begin, it then aligns everything following with the comment.
2598+
///
2599+
/// * No spaces inside the braced list. (Even for a comment at the first
2600+
/// position.)
2601+
/// * No line break before the closing brace.
2602+
/// * Indentation with the continuation indent.
2603+
///
2604+
/// \code
2605+
/// vector<int> x{1, 2, 3, 4};
2606+
/// vector<T> x{{}, {}, {}, {}};
2607+
/// f(MyMap[{composite, key}]);
2608+
/// new int[3]{1, 2, 3};
2609+
/// Type name{//Comment
2610+
/// value};
2611+
/// \endcode
2612+
CBLSS_ToBeNamed,
2613+
};
2614+
2615+
/// The style to handle braced lists.
25792616
/// \version 3.4
2580-
bool Cpp11BracedListStyle;
2617+
CppBracedListStyleStyle Cpp11BracedListStyle;
25812618

25822619
/// This option is **deprecated**. See ``DeriveLF`` and ``DeriveCRLF`` of
25832620
/// ``LineEnding``.

‎clang/lib/Format/BreakableToken.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,10 @@ BreakableStringLiteralUsingOperators::BreakableStringLiteralUsingOperators(
306306
// In Verilog, all strings are quoted by double quotes, joined by commas,
307307
// and wrapped in braces. The comma is always before the newline.
308308
assert(QuoteStyle == DoubleQuotes);
309-
LeftBraceQuote = Style.Cpp11BracedListStyle ? "{\"" : "{ \"";
310-
RightBraceQuote = Style.Cpp11BracedListStyle ? "\"}" : "\" }";
309+
LeftBraceQuote =
310+
Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block ? "{\"" : "{ \"";
311+
RightBraceQuote =
312+
Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block ? "\"}" : "\" }";
311313
Postfix = "\",";
312314
Prefix = "\"";
313315
} else {

‎clang/lib/Format/ContinuationIndenter.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
833833
auto IsOpeningBracket = [&](const FormatToken &Tok) {
834834
auto IsStartOfBracedList = [&]() {
835835
return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
836-
Style.Cpp11BracedListStyle;
836+
Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block;
837837
};
838838
if (Tok.isNoneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) &&
839839
!IsStartOfBracedList()) {
@@ -925,7 +925,12 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
925925
TT_TableGenDAGArgOpenerToBreak) &&
926926
!(Current.MacroParent && Previous.MacroParent) &&
927927
(Current.isNot(TT_LineComment) ||
928-
Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) &&
928+
(Previous.is(BK_BracedInit) &&
929+
(Style.Cpp11BracedListStyle == FormatStyle::CBLSS_ToBeNamed ||
930+
(Style.Cpp11BracedListStyle == FormatStyle::CBLSS_FunctionCall &&
931+
(!Previous.Previous ||
932+
Previous.Previous->isNoneOf(tok::identifier, tok::l_paren))))) ||
933+
Previous.is(TT_VerilogMultiLineListLParen)) &&
929934
!IsInTemplateString(Current)) {
930935
CurrentState.Indent = State.Column + Spaces;
931936
CurrentState.IsAligned = true;

‎clang/lib/Format/Format.cpp‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,20 @@ struct ScalarEnumerationTraits<FormatStyle::BreakTemplateDeclarationsStyle> {
304304
}
305305
};
306306

307+
template <>
308+
struct ScalarEnumerationTraits<FormatStyle::CppBracedListStyleStyle> {
309+
static void enumeration(IO &IO,
310+
FormatStyle::CppBracedListStyleStyle &Value) {
311+
IO.enumCase(Value, "Block", FormatStyle::CBLSS_Block);
312+
IO.enumCase(Value, "FunctionCall", FormatStyle::CBLSS_FunctionCall);
313+
IO.enumCase(Value, "ToBeNamed", FormatStyle::CBLSS_ToBeNamed);
314+
315+
// For backward compatibility.
316+
IO.enumCase(Value, "false", FormatStyle::CBLSS_Block);
317+
IO.enumCase(Value, "true", FormatStyle::CBLSS_ToBeNamed);
318+
}
319+
};
320+
307321
template <> struct ScalarEnumerationTraits<FormatStyle::DAGArgStyle> {
308322
static void enumeration(IO &IO, FormatStyle::DAGArgStyle &Value) {
309323
IO.enumCase(Value, "DontBreak", FormatStyle::DAS_DontBreak);
@@ -1628,7 +1642,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
16281642
LLVMStyle.CompactNamespaces = false;
16291643
LLVMStyle.ConstructorInitializerIndentWidth = 4;
16301644
LLVMStyle.ContinuationIndentWidth = 4;
1631-
LLVMStyle.Cpp11BracedListStyle = true;
1645+
LLVMStyle.Cpp11BracedListStyle = FormatStyle::CBLSS_ToBeNamed;
16321646
LLVMStyle.DerivePointerAlignment = false;
16331647
LLVMStyle.DisableFormat = false;
16341648
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
@@ -1904,7 +1918,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
19041918
// beneficial there. Investigate turning this on once proper string reflow
19051919
// has been implemented.
19061920
GoogleStyle.BreakStringLiterals = false;
1907-
GoogleStyle.Cpp11BracedListStyle = false;
1921+
GoogleStyle.Cpp11BracedListStyle = FormatStyle::CBLSS_Block;
19081922
GoogleStyle.SpacesInContainerLiterals = false;
19091923
} else if (Language == FormatStyle::LK_ObjC) {
19101924
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
@@ -2000,7 +2014,7 @@ FormatStyle getMozillaStyle() {
20002014
MozillaStyle.BreakTemplateDeclarations = FormatStyle::BTDS_Yes;
20012015
MozillaStyle.ConstructorInitializerIndentWidth = 2;
20022016
MozillaStyle.ContinuationIndentWidth = 2;
2003-
MozillaStyle.Cpp11BracedListStyle = false;
2017+
MozillaStyle.Cpp11BracedListStyle = FormatStyle::CBLSS_Block;
20042018
MozillaStyle.FixNamespaceComments = false;
20052019
MozillaStyle.IndentCaseLabels = true;
20062020
MozillaStyle.ObjCSpaceAfterProperty = true;
@@ -2023,7 +2037,7 @@ FormatStyle getWebKitStyle() {
20232037
Style.BreakBeforeBraces = FormatStyle::BS_WebKit;
20242038
Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
20252039
Style.ColumnLimit = 0;
2026-
Style.Cpp11BracedListStyle = false;
2040+
Style.Cpp11BracedListStyle = FormatStyle::CBLSS_Block;
20272041
Style.FixNamespaceComments = false;
20282042
Style.IndentWidth = 4;
20292043
Style.NamespaceIndentation = FormatStyle::NI_Inner;
@@ -2043,7 +2057,7 @@ FormatStyle getGNUStyle() {
20432057
Style.BreakBeforeBraces = FormatStyle::BS_GNU;
20442058
Style.BreakBeforeTernaryOperators = true;
20452059
Style.ColumnLimit = 79;
2046-
Style.Cpp11BracedListStyle = false;
2060+
Style.Cpp11BracedListStyle = FormatStyle::CBLSS_Block;
20472061
Style.FixNamespaceComments = false;
20482062
Style.KeepFormFeed = true;
20492063
Style.SpaceBeforeParens = FormatStyle::SBPO_Always;

‎clang/lib/Format/FormatToken.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ bool FormatToken::isTypeOrIdentifier(const LangOptions &LangOpts) const {
6666

6767
bool FormatToken::isBlockIndentedInitRBrace(const FormatStyle &Style) const {
6868
assert(is(tok::r_brace));
69-
if (!Style.Cpp11BracedListStyle ||
69+
if (Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block ||
7070
Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent) {
7171
return false;
7272
}
@@ -88,7 +88,8 @@ bool FormatToken::opensBlockOrBlockTypeList(const FormatStyle &Style) const {
8888
return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||
8989
(is(tok::l_brace) &&
9090
(getBlockKind() == BK_Block || is(TT_DictLiteral) ||
91-
(!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
91+
(Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block &&
92+
NestingLevel == 0))) ||
9293
(is(tok::less) && Style.isProto());
9394
}
9495

@@ -184,7 +185,8 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
184185
// In C++11 braced list style, we should not format in columns unless they
185186
// have many items (20 or more) or we allow bin-packing of function call
186187
// arguments.
187-
if (Style.Cpp11BracedListStyle && !Style.BinPackArguments &&
188+
if (Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block &&
189+
!Style.BinPackArguments &&
188190
(Commas.size() < 19 || !Style.BinPackLongBracedList)) {
189191
return;
190192
}
@@ -228,7 +230,7 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
228230
ItemEnd = Token->MatchingParen;
229231
const FormatToken *NonCommentEnd = ItemEnd->getPreviousNonComment();
230232
ItemLengths.push_back(CodePointsBetween(ItemBegin, NonCommentEnd));
231-
if (Style.Cpp11BracedListStyle &&
233+
if (Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block &&
232234
!ItemEnd->Previous->isTrailingComment()) {
233235
// In Cpp11 braced list style, the } and possibly other subsequent
234236
// tokens will need to stay on a line with the last element.

‎clang/lib/Format/TokenAnnotator.cpp‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4096,16 +4096,9 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
40964096
for (auto *Current = First->Next; Current; Current = Current->Next) {
40974097
const FormatToken *Prev = Current->Previous;
40984098
if (Current->is(TT_LineComment)) {
4099-
if (Prev->is(BK_BracedInit) && Prev->opensScope()) {
4100-
Current->SpacesRequiredBefore =
4101-
(Style.Cpp11BracedListStyle && !Style.SpacesInParensOptions.Other)
4102-
? 0
4103-
: 1;
4104-
} else if (Prev->is(TT_VerilogMultiLineListLParen)) {
4105-
Current->SpacesRequiredBefore = 0;
4106-
} else {
4107-
Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
4108-
}
4099+
Current->SpacesRequiredBefore = Prev->is(TT_VerilogMultiLineListLParen)
4100+
? 0
4101+
: Style.SpacesBeforeTrailingComments;
41094102

41104103
// If we find a trailing comment, iterate backwards to determine whether
41114104
// it seems to relate to a specific parameter. If so, break before that
@@ -4449,8 +4442,10 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
44494442
(Left.ParameterCount <= 1 || Style.AllowAllArgumentsOnNextLine)) {
44504443
return 0;
44514444
}
4452-
if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
4445+
if (Left.is(tok::l_brace) &&
4446+
Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block) {
44534447
return 19;
4448+
}
44544449
return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
44554450
: 19;
44564451
}
@@ -4616,7 +4611,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
46164611
// Format empty list as `<>`.
46174612
if (Left.is(tok::less) && Right.is(tok::greater))
46184613
return false;
4619-
return !Style.Cpp11BracedListStyle;
4614+
return Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block;
46204615
}
46214616
// Don't attempt to format operator<(), as it is handled later.
46224617
if (Right.isNot(TT_OverloadedOperatorLParen))
@@ -4784,7 +4779,8 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
47844779
const auto SpaceRequiredForArrayInitializerLSquare =
47854780
[](const FormatToken &LSquareTok, const FormatStyle &Style) {
47864781
return Style.SpacesInContainerLiterals ||
4787-
(Style.isProto() && !Style.Cpp11BracedListStyle &&
4782+
(Style.isProto() &&
4783+
Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block &&
47884784
LSquareTok.endsSequence(tok::l_square, tok::colon,
47894785
TT_SelectorName));
47904786
};
@@ -4817,7 +4813,8 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
48174813
if ((Left.is(tok::l_brace) && Left.isNot(BK_Block)) ||
48184814
(Right.is(tok::r_brace) && Right.MatchingParen &&
48194815
Right.MatchingParen->isNot(BK_Block))) {
4820-
return !Style.Cpp11BracedListStyle || Style.SpacesInParensOptions.Other;
4816+
return Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block ||
4817+
Style.SpacesInParensOptions.Other;
48214818
}
48224819
if (Left.is(TT_BlockComment)) {
48234820
// No whitespace in x(/*foo=*/1), except for JavaScript.
@@ -4999,7 +4996,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
49994996
Left.Children.empty()) {
50004997
if (Left.is(BK_Block))
50014998
return Style.SpaceInEmptyBraces != FormatStyle::SIEB_Never;
5002-
if (Style.Cpp11BracedListStyle) {
4999+
if (Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block) {
50035000
return Style.SpacesInParens == FormatStyle::SIPO_Custom &&
50045001
Style.SpacesInParensOptions.InEmptyParentheses;
50055002
}
@@ -5081,7 +5078,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
50815078
if (Left.MatchingParen &&
50825079
Left.MatchingParen->is(TT_ProtoExtensionLSquare) &&
50835080
Right.isOneOf(tok::l_brace, tok::less)) {
5084-
return !Style.Cpp11BracedListStyle;
5081+
return Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block;
50855082
}
50865083
// A percent is probably part of a formatting specification, such as %lld.
50875084
if (Left.is(tok::percent))
@@ -5521,7 +5518,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
55215518
if (Left.is(tok::greater) && Right.is(tok::greater)) {
55225519
if (Style.isTextProto() ||
55235520
(Style.Language == FormatStyle::LK_Proto && Left.is(TT_DictLiteral))) {
5524-
return !Style.Cpp11BracedListStyle;
5521+
return Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block;
55255522
}
55265523
return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
55275524
((Style.Standard < FormatStyle::LS_Cpp11) ||
@@ -6382,7 +6379,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
63826379
return false;
63836380
}
63846381
if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
6385-
!Style.Cpp11BracedListStyle) {
6382+
Style.Cpp11BracedListStyle == FormatStyle::CBLSS_Block) {
63866383
return false;
63876384
}
63886385
if (Left.is(TT_AttributeLParen) ||

‎clang/lib/Format/WhitespaceManager.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,7 +1322,8 @@ void WhitespaceManager::alignArrayInitializersRightJustified(
13221322
if (!CellDescs.isRectangular())
13231323
return;
13241324

1325-
const int BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
1325+
const int BracePadding =
1326+
Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block ? 0 : 1;
13261327
auto &Cells = CellDescs.Cells;
13271328
// Now go through and fixup the spaces.
13281329
auto *CellIter = Cells.begin();
@@ -1398,7 +1399,8 @@ void WhitespaceManager::alignArrayInitializersLeftJustified(
13981399
if (!CellDescs.isRectangular())
13991400
return;
14001401

1401-
const int BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
1402+
const int BracePadding =
1403+
Style.Cpp11BracedListStyle != FormatStyle::CBLSS_Block ? 0 : 1;
14021404
auto &Cells = CellDescs.Cells;
14031405
// Now go through and fixup the spaces.
14041406
auto *CellIter = Cells.begin();

‎clang/unittests/Format/ConfigParseTest.cpp‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,6 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
176176
CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
177177
CHECK_PARSE_BOOL(BreakStringLiterals);
178178
CHECK_PARSE_BOOL(CompactNamespaces);
179-
CHECK_PARSE_BOOL(Cpp11BracedListStyle);
180179
CHECK_PARSE_BOOL(DerivePointerAlignment);
181180
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
182181
CHECK_PARSE_BOOL(DisableFormat);
@@ -1139,6 +1138,18 @@ TEST(ConfigParseTest, ParsesConfiguration) {
11391138
FormatStyle::SDS_Leave);
11401139
CHECK_PARSE("SeparateDefinitionBlocks: Never", SeparateDefinitionBlocks,
11411140
FormatStyle::SDS_Never);
1141+
1142+
CHECK_PARSE("Cpp11BracedListStyle: FunctionCall", Cpp11BracedListStyle,
1143+
FormatStyle::CBLSS_FunctionCall);
1144+
CHECK_PARSE("Cpp11BracedListStyle: Block", Cpp11BracedListStyle,
1145+
FormatStyle::CBLSS_Block);
1146+
CHECK_PARSE("Cpp11BracedListStyle: ToBeNamed", Cpp11BracedListStyle,
1147+
FormatStyle::CBLSS_ToBeNamed);
1148+
// For backward compatibility:
1149+
CHECK_PARSE("Cpp11BracedListStyle: false", Cpp11BracedListStyle,
1150+
FormatStyle::CBLSS_Block);
1151+
CHECK_PARSE("Cpp11BracedListStyle: true", Cpp11BracedListStyle,
1152+
FormatStyle::CBLSS_ToBeNamed);
11421153
}
11431154

11441155
TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {

0 commit comments

Comments
 (0)