[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

Anastasiia Lukianenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 22 04:18:32 PDT 2021


anastasiia_lukianenko added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5075
+               "};",
+               Style);
+}
----------------
MyDeveloperDay wrote:
> I wonder if there should be other options
> 
> i.e.
> 
> ```
> struct new_struct struct_name = {
>     a = 1
> };
> ```
> ```
> struct new_struct struct_name = { a = 1 };
> ```
> 
> ```
> struct new_struct struct_name = 
> { 
>     a = 1,
> };
> ```
> 
> Again I don't want to discourage you, but this is very much related to one of the key problem areas of clang-format that it really doesn't do well, so its one of the places users ALWAY have to add //clang-format off
> 
> e.g.
> 
> ```
> Status GradForBinaryCwise(FunctionDef* g, std::vector<FDH::Node> body) {
>   // clang-format off
>   std::vector<FDH::Node> nodes = {
>     {{"sx"}, "Shape", {"x"}},
>     {{"sy"}, "Shape", {"y"}},
>   };
>   // clang-format on
> ```
> 
> If we are going to fix this to some extent we should fix it for all those cases too. I understand that that might be beyond what you are trying to achieve, but I think its important that we explore what might be needed to satisfy being able to get rid of all these //clang-format off cases as some point and you've woken the beast in this area ;-)
> 
> To be honest fixing this is one of biggest things (and very commendable if you want to focus on it) and whilst your fix is along the right lines, it only addresses the struct cases (and not the class case example (as above) and it perhaps doesn't cover where there might be something before the struct as in 
> 
> ```
> static struct X Y = {
> }
> ```
> 
> If your interested in pursuing this still, I'm happy to try and help you walk through it, where I can. However you'll have to bare with me as it might be beyond my skill set.
> 
> 
I understand what you mean, but at the moment options such as "AfterStruct" are also implemented with checking the first word in a line (not considering cases with "static" word). It seems to me that the option that I add should only concern structures, and new configurations  should be created for the class, etc. (BreakBeforeClassInit, BreakBeforeEnumInit), so that the formatting would be more flexible.


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

https://reviews.llvm.org/D91949



More information about the cfe-commits mailing list