This is an archive of the discontinued LLVM Phabricator instance.

[ClangFormat] Add new style option IndentGotoLabels
ClosedPublic

Authored by tetsuo-cpp on Aug 31 2019, 4:05 AM.

Details

Summary

This option determines whether goto labels are indented according to scope. Setting this option to false causes goto labels to be flushed to the left.
This is mostly copied from this patch submitted by Christian Neukirchen that didn't make its way into trunk.

true:                                  false:
int f() {                      vs.     int f() {
  if (foo()) {                           if (foo()) {
  label1:                              label1:
    bar();                                 bar();
  }                                      }
label2:                                label2:
  return 1;                              return 1;
}                                      }

Diff Detail

Repository
rL LLVM

Event Timeline

tetsuo-cpp created this revision.Aug 31 2019, 4:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2019, 4:05 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Friendly ping. Could I please get this looked at?

MyDeveloperDay accepted this revision.Sep 7 2019, 5:01 AM
MyDeveloperDay added a project: Restricted Project.

This LGTM, just some minor Nits

clang/lib/Format/UnwrappedLineParser.cpp
1354 ↗(On Diff #218212)

could this just be

 // Align goto labels.
parseLabel(!Style.IndentGotoLabels);
1973 ↗(On Diff #218212)

I think this could just be LeftAlignLabel, I'm not sure this function needs to know anything about goto specifically

1978 ↗(On Diff #218212)

then this would just be

if (LeftAlignLabel){
    Line->level = 0;
}
clang/lib/Format/UnwrappedLineParser.h
109 ↗(On Diff #218212)

Nit: maybe rename to just LeftAlignLabel

This revision is now accepted and ready to land.Sep 7 2019, 5:02 AM

Addressed review comments.

@MyDeveloperDay
Thanks for your suggestions! I think it's better now.
This is my first LLVM patch (hopefully first of many) so I don't have commit permissions yet. If you're still happy with this change, could you please commit on my behalf?

tetsuo-cpp marked 4 inline comments as done.Sep 10 2019, 12:27 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 12 2019, 3:06 AM