This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix another bug in AlignConsecutiveAssignments
ClosedPublic

Authored by owenpan on May 4 2022, 12:49 PM.

Details

Summary

Similar to D124868.

The ShouldShiftBeAdded lambda checks if extra space should be
added before the wrapped part of a braced list. If the first
element of the list is wrapped, no extra space should be added.

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

Diff Detail

Event Timeline

owenpan created this revision.May 4 2022, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2022, 12:49 PM
owenpan requested review of this revision.May 4 2022, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2022, 12:49 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This revision is now accepted and ready to land.May 4 2022, 12:51 PM
curdeius accepted this revision.May 4 2022, 2:21 PM

LGTM!

Just one last thought, shouldn't we have a test case close to what was reported in the issue? I.e. with PP directives.

No, because the first part of that test case would cause non-consecutive assignments to be aligned. (See https://github.com/llvm/llvm-project/issues/55265.) Only then would the if block be treated as a braced list and get misindented. We should definitely use test cases with PP directives when fixing #55265 though.