[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 7 05:02:39 PDT 2019


MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

This LGTM, just some minor Nits



================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1354
           Line->Tokens.begin()->Tok->MustBreakBefore = true;
-          parseLabel();
+          parseLabel(true);
           return;
----------------
could this just be 


```
 // Align goto labels.
parseLabel(!Style.IndentGotoLabels);
```






================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1973
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool GotoLabel) {
   nextToken();
----------------
I think this could just be `LeftAlignLabel`, I'm not sure this function needs to know anything about `goto` specifically


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1978
     --Line->Level;
+  if (GotoLabel && !Style.IndentGotoLabels)
+    Line->Level = 0;
----------------
then this would just be 


```
if (LeftAlignLabel){
    Line->level = 0;
}
```


================
Comment at: clang/lib/Format/UnwrappedLineParser.h:109
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool GotoLabel = false);
   void parseCaseLabel();
----------------
Nit: maybe rename to just `LeftAlignLabel`


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037





More information about the cfe-commits mailing list