This is an archive of the discontinued LLVM Phabricator instance.

[clang-format][NFC] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.
ClosedPublic

Authored by jp4a50 on Mar 27 2023, 12:29 PM.

Details

Summary

This is a non-functional change which acts as a precursor to the functional changes (fixes) in https://reviews.llvm.org/D146101, pulled out as requested by @MyDeveloperDay .

The changes:

  • make the tests more concise
  • fix invalid C++ in the code samples
  • ensure line breaks in tests' code samples correspond to line breaks in the test code itself for the avoidance of confusion

Diff Detail

Event Timeline

jp4a50 created this revision.Mar 27 2023, 12:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2023, 12:29 PM
jp4a50 requested review of this revision.Mar 27 2023, 12:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2023, 12:29 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jp4a50 edited the summary of this revision. (Show Details)Mar 27 2023, 12:31 PM
jp4a50 added reviewers: MyDeveloperDay, owenpan.
jp4a50 added a subscriber: MyDeveloperDay.
jp4a50 added inline comments.Mar 27 2023, 12:34 PM
clang/unittests/Format/FormatTest.cpp
22020

The refactored version of these test cases has been moved up to immediately follow the setting of OuterScope so that the result can be compared with the identical code sample above which doesn't use OuterScope. It was odd that these weren't grouped together before.

22036

This is the comment that I believe is calling out the seemingly broken formatting behaviour in the test case directly above it. I think it's pretty clear that the code in that test case should not be formatted that way when OuterScope is set (or even without it - the formatting just doesn't make any sense really).

owenpan retitled this revision from [clang-format] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option. to [clang-format][NFC] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option..Mar 27 2023, 1:23 PM
owenpan added a project: Restricted Project.
MyDeveloperDay accepted this revision.Mar 27 2023, 3:21 PM
This revision is now accepted and ready to land.Mar 27 2023, 3:21 PM
jp4a50 added a comment.EditedMar 28 2023, 7:51 AM

If you guys are happy with this, could you please merge it for me?

Edit: Also could you please advise about the failing CI test? It doesn't look like it's anything to do with my changes but do let me know if that's not the case.

If you guys are happy with this, could you please merge it for me?

You need to state a name and email for the commit.

Edit: Also could you please advise about the failing CI test? It doesn't look like it's anything to do with my changes but do let me know if that's not the case.

If you have checked your code does compile and run on your side and the reported error is not in clang-format you can just ignore it.

> You need to state a name and email for the commit.

Name: Jon Phillips
Email: jonap2811@gmail.com

Also, thanks for relating the child diff to this one. I couldn't see how to do it at first but have seen the option now.

This revision was landed with ongoing or failed builds.Apr 1 2023, 7:36 PM
This revision was automatically updated to reflect the committed changes.