This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.
ClosedPublic

Authored by curdeius on Jan 6 2022, 2:13 PM.

Details

Summary

Fixes https://github.com/llvm/llvm-project/issues/32031.

Before this change, BraceWrapping: AfterFunction would affect synchronized blocks in Java, but they should be formatted w.r.t. BraceWrapping: AfterControlStatement.

Using the config:

BreakBeforeBraces: Custom
BraceWrapping:
  AfterControlStatement: false
  AfterFunction: true

would result in misformatted code like:

class Foo {
  void bar()
  {
    synchronized (this)
    {
      a();
      a();
    }
  }
}

instead of:

class Foo {
  void bar()
  {
    synchronized (this) {
      a();
      a();
    }
  }
}

Diff Detail

Event Timeline

curdeius requested review of this revision.Jan 6 2022, 2:13 PM
curdeius created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2022, 2:13 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
owenpan accepted this revision.Jan 7 2022, 12:08 AM

LGTM except for the nits.

clang/lib/Format/UnwrappedLineParser.cpp
1528

Remove redundant parens.

1535

Add braces to "keep it uniform" with the if block.

This revision is now accepted and ready to land.Jan 7 2022, 12:08 AM

Thanks for the reviews!

clang/lib/Format/UnwrappedLineParser.cpp
1528

I don't think they're redundant because of 2 reasons: nested if and comment inside.
Cf. https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.

1535

Will do.

owenpan added inline comments.Jan 7 2022, 12:51 AM
clang/lib/Format/UnwrappedLineParser.cpp
1528

By "parens" I meant ( and ). :)

curdeius added inline comments.Jan 7 2022, 12:54 AM
clang/lib/Format/UnwrappedLineParser.cpp
1528

Sorry, my bad.