This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Correctly limit formatted ranges when specifying qualifier alignment
ClosedPublic

Authored by cogilvie on May 2 2023, 3:20 AM.

Details

Summary

The qualifier alignment fixer appeared to ignore any ranges specified for limiting formatting.
This change ensures that it only formats affected lines to avoid unexpected changes as reported by:
https://github.com/llvm/llvm-project/issues/54888

Diff Detail

Event Timeline

cogilvie created this revision.May 2 2023, 3:20 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 2 2023, 3:20 AM
cogilvie requested review of this revision.May 2 2023, 3:20 AM
HazardyKnusperkeks retitled this revision from Correctly limit formatted ranges when specifying qualifier alignment to [clang-format] Correctly limit formatted ranges when specifying qualifier alignment.
This revision is now accepted and ready to land.May 2 2023, 12:19 PM
owenpan accepted this revision.May 2 2023, 3:29 PM
owenpan added inline comments.
clang/unittests/Format/QualifierFixerTest.cpp
1351
1352–1357

Please use verifyFormat instead of EXPECT_EQ here and below.

1359–1360
owenpan requested changes to this revision.May 2 2023, 3:29 PM
This revision now requires changes to proceed.May 2 2023, 3:29 PM
cogilvie updated this revision to Diff 519007.May 3 2023, 1:25 AM
cogilvie updated this revision to Diff 519008.May 3 2023, 1:27 AM

Thanks for the patch...this tells me people are starting to use this feature in anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which means you have the code modifying features perhaps on my default...

LGTM, if you will need someone to land this for you we'll need your name and email address, if not please wait for @owenpan

MyDeveloperDay accepted this revision.May 3 2023, 10:31 AM

Please mark review comments as done after addressing them.

clang/unittests/Format/QualifierFixerTest.cpp
1356
1366
cogilvie updated this revision to Diff 519389.May 4 2023, 2:01 AM
cogilvie marked 3 inline comments as done.
cogilvie marked 2 inline comments as done.May 4 2023, 2:06 AM

Fixed review comments

Thanks for the patch...this tells me people are starting to use this feature in anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which means you have the code modifying features perhaps on my default...

LGTM, if you will need someone to land this for you we'll need your name and email address, if not please wait for @owenpan

Thanks for the reviews.

Yes formatting modifications by default seems to work well when working on older codebases where applying changes to the whole file hides the real intent of a change.

Please submit this for me with the details Colin Ogilvie colin.ogilvie@kdab.com

owenpan accepted this revision.May 4 2023, 2:44 AM
This revision is now accepted and ready to land.May 4 2023, 2:44 AM
This revision was landed with ongoing or failed builds.May 4 2023, 2:59 AM
This revision was automatically updated to reflect the committed changes.