[PATCH] D130299: [clang-format] FIX: Misformatting lambdas with trailing return type 'auto' in braced lists

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.
This revision now requires changes to proceed.

In D130299#3672486 <https://reviews.llvm.org/D130299#3672486>, @denis-fatkulin wrote:

>> Could you please add full git context?
>
> I updated the patch with properly git context. Thanks!
>
>> Was the problem due to misannotation of auto? If so, could you add an annotator test?
>
> I'm not sure about the questions. I will try to explain my patch purpose. Actually there was two problems:
>
> 1. `auto` wasn't detected as properly type keyword in lambda's trailing return type. So, formatting was completely wrong for this case (fixed in `clang/lib/Format/UnwrappedLineParser.cpp`)
> 2. The keyword `auto` and left brace `{` was interpreted as declaration `auto{}`. So, formatting delete a space symbol between them. (fixed in `clang/lib/Format/TokenAnnotator.cpp`)
>
> Both cases are checked in changes at `clang/unittests/Format/FormatTest.cpp` and I think the unit test is sufficient.

The question is, wether `auto` did get the type `kw_auto` before your change or not. If not a test for the token annotator would be in place. See e.g. D129946 <https://reviews.llvm.org/D129946>.
And I actually think you should amend the test case from there, because as far as I can see if there was an `auto` we would stop parsing the lambda and not assign the `TT_LambdaLBrace`, etc. In fact I can't see the anything related to braced lists in your fixes. The error was primarily an annotation error, with the adding space as a formation error. The fucked up braced list is just a follow up error.



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3314
+  // Lambda with trailing return type 'auto': []() -> auto { return T; }
+  if (Left.is(tok::kw_auto) && Right.getType() == TT_LambdaLBrace)
+    return true;
----------------



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3314
+  // Lambda with trailing return type 'auto': []() -> auto { return T; }
+  if (Left.is(tok::kw_auto) && Right.getType() == TT_LambdaLBrace)
+    return true;
----------------
HazardyKnusperkeks wrote:
> 
Maybe split it up in two changes, and change it to this, because I think we would have the same problem.


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

https://reviews.llvm.org/D130299