Page MenuHomePhabricator

[Format/ObjC] Avoid breaking between unary operators and operands
ClosedPublic

Authored by benhamilton on Jul 15 2019, 3:12 PM.

Details

Summary

Test Plan:

New tests added. Ran tests with:
% ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
Confirmed tests failed before change and passed after change.

Diff Detail

Repository
rL LLVM

Event Timeline

benhamilton created this revision.Jul 15 2019, 3:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 3:12 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
sammccall added inline comments.
clang/lib/Format/TokenAnnotator.cpp
2427–2428 ↗(On Diff #209968)

This looks a little suspicious only because it's so specific.

It seems like you'd *always* want a harsh penalty for breaking between a unary operator and its operand. @klimek does this look right, should we drop the ObcJMethodExpr requirement or is this case handled elsewhere?

benhamilton marked an inline comment as done.
  • Rebase.
  • Change to just avoid breaking when the left-hand side is a unary operator
benhamilton marked an inline comment as done.Jul 16 2019, 7:36 AM
benhamilton added inline comments.
clang/lib/Format/TokenAnnotator.cpp
2427–2428 ↗(On Diff #209968)

I tried changing it to just if (Left.is(TT_UnaryOperator)) and all the tests passed. I'll go for the gusto and assume this is fine.

benhamilton marked an inline comment as done.Jul 17 2019, 3:50 PM

Any more thoughts on this?

sammccall accepted this revision.Jul 19 2019, 5:04 AM

Yeah, worst case we'll need to roll this back and go with the targeted fix, but I don't see a problem.

This revision is now accepted and ready to land.Jul 19 2019, 5:04 AM
benhamilton retitled this revision from [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations to [Format/ObjC] Avoid breaking between unary operators and operands.Jul 19 2019, 9:46 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2019, 9:50 AM