[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

Jon Phillips via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 15 04:52:25 PDT 2023


jp4a50 added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3540
 
+       someMethod(someOtherMethod(
+           [](SomeReallyLongLambdaSignatureArgument foo) {
----------------
MyDeveloperDay wrote:
> This code may come from Format.h
Sorry, I'm not sure I understand.

Do you mean that this code should *also* be added to the corresponding documentation in `Format.h`? Or do you mean that the documentation in this file is programatically generated from `Format.h` and I should therefore only put it there? Or something else?


================
Comment at: clang/unittests/Format/FormatTest.cpp:21916
 
   // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(60);
----------------
MyDeveloperDay wrote:
> Why are you changing existing tests
Sorry about the rather large diff here.

I have made the following changes to existing tests (all for what I think are good reasons):

  - Reduced the column limit and then reduced the verbosity of the code samples in the test to make them more readable. Before the column limit was 100 and the code samples were very long. I found this made them unnecessarily difficult to read, especially because some of the lines of the code sample would wrap before a newline in the code itself, which was confusing.
  - Changed a number of tests written in the form `EXPECT_EQ(code, format(otherCode))` to simply `verifyFormat(code)` because the latter is much shorter and easier to read. If you can think of a good reason to favour the former over the latter then let me know but it just seemed like unnecessary duplication to me.
  - Some of the tests weren't syntactically valid C++ e.g. the tests at line 21939 which I have changed to have a valid (rather than incomplete) trailing return type.
  - The macro test actually captured a bug so I had to change the code sample there to reflect the now fixed behaviour.

I also added one or two more tests at the bottom to capture more complex cases like when more than one argument to a function call is a lambda. 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146042/new/

https://reviews.llvm.org/D146042



More information about the cfe-commits mailing list