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

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 7 10:14:34 PST 2020


MyDeveloperDay added inline comments.


================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:979
+    ContinuationIndent += Style.ContinuationIndentWidth;
   const FormatToken *PreviousNonComment = Current.getPreviousNonComment();
   const FormatToken *NextNonComment = Previous.getNextNonComment();
----------------
I personally can't tell if this is the correct place to do this, was there a reason why you couldn't add it to `mustBreakBefore()`?


================
Comment at: clang/unittests/Format/FormatTest.cpp:5075
+               "};",
+               Style);
+}
----------------
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.




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

https://reviews.llvm.org/D91949



More information about the cfe-commits mailing list