[clang-format] FIX: Misannotation 'auto' as trailing return type in lambdas

[denis-fatkulin]

Summary

Lambdas with trailing return type 'auto' are annotated incorrectly. It causes a misformatting. The simpliest code to reproduce is:

auto list = {[]() -> auto { return 0; }};

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

[curdeius]

Could you please add full git context?
Was the problem due to misannotation of auto? If so, could you add an annotator test?

[denis-fatkulin]

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.

[HazardyKnusperkeks]

In 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.
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.

clang/lib/Format/TokenAnnotator.cpp

3314

3314

Maybe split it up in two changes, and change it to this, because I think we would have the same problem.

[denis-fatkulin]

Patch is splitted, as suggested by @HazardyKnusperkeks

This piece of changes is related to misannotation KW 'auto' as trailing return type in lambdas. Also, appropriate test is added.

[HazardyKnusperkeks]

Thanks for the changes.

clang/unittests/Format/TokenAnnotatorTest.cpp

752 ↗

(On Diff #447052)

I'd just merge it with the test above.

[owenpan]

clang/unittests/Format/TokenAnnotatorTest.cpp

752 ↗

(On Diff #447052)

I'd just merge it with the test above.

+1.

753–756 ↗

(On Diff #447052)

Should we add a test case with an & between auto and {?

[curdeius]

clang/unittests/Format/TokenAnnotatorTest.cpp

752 ↗

(On Diff #447052)

[denis-fatkulin]

Merrged with previous test case. Cases for auto & and auto * are added also.

[HazardyKnusperkeks]

Do you have commit access, or do you want to apply for it, or do you need someone to push the change(s)? If the latter we need a name and mail for the commit.

[denis-fatkulin]

In D130299#3674586, @HazardyKnusperkeks wrote:

Do you have commit access, or do you want to apply for it, or do you need someone to push the change(s)? If the latter we need a name and mail for the commit.

I haven't access for merging changes to code base. So, I need support from community for doing this.

My name and mail:
Name: Denis Fatkulin
Mail: [e-mail]

Thank you!

PS. Related patch should be merged too. D130417

clang/lib/Format/TokenAnnotator.cpp

     }
   }
 
+  // Lambda with trailing return type 'auto': []() -> auto { return T; }
+  if (Left.is(tok::kw_auto) && Right.getType() == TT_LambdaLBrace)
+    return true;
+
   // auto{x} auto(x)
   if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
     return false;

clang/lib/Format/UnwrappedLineParser.cpp

     case tok::l_square:
       parseSquare();
       break;
+    case tok::kw_auto:
     case tok::kw_class:
     case tok::kw_template:
     case tok::kw_typename:

clang/unittests/Format/FormatTest.cpp

   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, LambdaWithTrailingAutoInBracedList) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("auto list = {[]() -> auto { return Val; }};", Style);
+  verifyFormat("auto list = {[]() -> auto * { return Ptr; }};", Style);
+  verifyFormat("auto list = {[]() -> auto & { return Ref; }};", Style);
+}
+
 TEST_F(FormatTest, SpacesInConditionalStatement) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.IfMacros.clear();