[PATCH] D91949: [clang-format] Add BreakBeforeStructInitialization configuration

[Anastasiia Lukianenko via Phabricator via cfe-commits]

anastasiia_lukianenko added a comment.

Thanks for the detailed review. After more detailed testing, I will upload a new version of the patches, which will fix configuration influence on other formatting options.



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3492
   const FormatToken &Left = *Right.Previous;
+   if (Style.BreakBeforeStructInitialization && Right.is(tok::l_brace) &&
+       (Right.is(BK_BracedInit) || Left.is(tok::equal)))
----------------
MyDeveloperDay wrote:
> This feels quite general and as its high up in this function I'm thinking its going to get hit alot...
> 
> To me it feels like any construct like this could potentially fall into this trap correct
> 
> ` = { `
> 
> what would happen to
> 
> `std::vector<int> v = {2, 1};`
> 
> did the FormatTests run cleanly, (assuming they did wow!)
Thank you for paying attention to this problem. I tested these configurations mostly on C code and now I see that it has undefined behavior in other languages... I think this condition must have more specific parameters that will format the code only for structures.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5052
+  Style.ColumnLimit = 80;
+  Style.BreakBeforeStructInitialization = true;
+  verifyFormat("struct new_struct struct_name = \n"
----------------
MyDeveloperDay wrote:
> This is never initialized in the main code? so what value will it have by default?
I'll improve tests and upload new version. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91949/new/

https://reviews.llvm.org/D91949