[clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

[ChuanqiXu]

Summary

The clang-format may go wrong when handle c++ coroutine keywords and pointer.
The default value for PointerAlignment is PAS_Right. So the following format is good:

co_return *a;

But within some code style, the value for PointerAlignment is PAS_Left, the behavior goes wrong:

co_return* a;

test-plan: check-llvm, check-clang

[ChuanqiXu]

@djasper gental ping~

[MyDeveloperDay]

What does the behavior goes wrong mean? why can't it be left aligned?

[• Quuxplusone]

Peanut gallery says: It seems to me that your root problem here is that co_yield is not being recognized by the parser as a keyword, and so you're seeing e.g. co_yield* p; the same way you'd see CoYield* p;.
But in that case, you should also be seeing co_yield -1; formatted as co_yield - 1; the same way you'd see CoYield - 1;.
Is it possible that you're not using the right (C++20) language mode? (OTOH, see D69144, D69180, which also didn't concern themselves with language modes. I don't understand why not.)

Could you please re-upload the diff with context (i.e. git show -U999)?

[ChuanqiXu]

In D91245#2437518, @MyDeveloperDay wrote:

What does the behavior goes wrong mean? why can't it be left aligned?

Because when we use PointerAlignment attribute, I think we mean that the pointer need to be aligned in declarations. However, co_return *a; is not a declaration.

[ChuanqiXu]

Re-upload patch with context

[ChuanqiXu]

In D91245#2437741, @Quuxplusone wrote:

Peanut gallery says: It seems to me that your root problem here is that co_yield is not being recognized by the parser as a keyword, and so you're seeing e.g. co_yield* p; the same way you'd see CoYield* p;.
But in that case, you should also be seeing co_yield -1; formatted as co_yield - 1; the same way you'd see CoYield - 1;.
Is it possible that you're not using the right (C++20) language mode? (OTOH, see D69144, D69180, which also didn't concern themselves with language modes. I don't understand why not.)

Could you please re-upload the diff with context (i.e. git show -U999)?

Yes, it looks like that co_yield is not being recognized as keywords. I use the Auto for Standard attribute in .clang-format config file. And co* would be recognized as keywords. From my point of view, I first find that the keyword return was processed specifically in the line 1962 in TokenAnnotator.cpp which recognize return as Unary Operator. So I choose to recognize co* keywords as Unary Operators to solve the format problem.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

7766

maybe add, just to clarify it.

verifyFormat("return *a", PASLeftStyle);

[ChuanqiXu]

add verifyFormat("return *a", PASLeftStyle); to clarify the change

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

7759–7761

I don't think this should be in the tests, because it is not true (if your patch works).

[ChuanqiXu]

clang/unittests/Format/FormatTest.cpp

7759–7761

I'm confusing about your comment. What is not true? Do you mean if my patch works then the co_return *a; should be formatted as co_return* a?

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

7759–7761

No, I refer to the comment. You say would mis-format, but it wouln't because of your change, or not? Otherwise the tests would not pass.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

7759–7761

[ChuanqiXu]

Edit the comment to avoid misleading.

clang/unittests/Format/FormatTest.cpp

7759–7761

Oh, I got it. Thanks!

[MyDeveloperDay]

Aside: Would you be prepared to take a look at D34225: [clang-format] Teach clang-format how to handle C++ coroutines which is pretty much been abandoned, and see if there is anything there that might help the co_routinues support?

[ChuanqiXu]

In D91245#2454504, @MyDeveloperDay wrote:

Aside: Would you be prepared to take a look at D34225: [clang-format] Teach clang-format how to handle C++ coroutines which is pretty much been abandoned, and see if there is anything there that might help the co_routinues support?

I think the test cases in D34225: [clang-format] Teach clang-format how to handle C++ coroutines are rare in our codes. It is a little strange to write co_return (43) instead of co_return 43. And the grammar for co_await (auto x : range) is used rarely. I think both of them are defects of clang-format while they are really rare in actual environments.

[MyDeveloperDay]

not that rare it seems

https://github.com/search?q=%22co_return+%28%22&type=code
https://github.com/search?q=%22for+co_await+%28%22&type=code

[MyDeveloperDay]

I don't like seeing users having to use `// clang-format off```

auto try_sequence = [](int& ref) -> return_ignore {
            /* clang-format off */
            for co_await(int v : return_random_int())
                ref = v;
            /* clang-format on*/
        };

it would be nice if we could land a co_routines improvement

[ChuanqiXu]

In D91245#2454610, @MyDeveloperDay wrote:

I don't like seeing users having to use `// clang-format off```

auto try_sequence = [](int& ref) -> return_ignore {
            /* clang-format off */
            for co_await(int v : return_random_int())
                ref = v;
            /* clang-format on*/
        };

it would be nice if we could land a co_routines improvement

From the links, I find most of them are test cases of LLVM projects. But I agree with you that /* clang-format off */ looks ugly. And I find that the grammar for co_await () is missing in newer standard documentation (N4849) while it was there in N4680. Maybe it is necessary to check whether this grammar is abandoned.

clang/lib/Format/TokenAnnotator.cpp

TokenType determineStarAmpUsage(const FormatToken &Tok, bool IsExpression,
     if (PrevToken->is(tok::coloncolon))
       return TT_PointerOrReference;
 
     if (PrevToken->is(tok::r_paren) && PrevToken->is(TT_TypeDeclarationParen))
       return TT_PointerOrReference;
 
     if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
                            tok::comma, tok::semi, tok::kw_return, tok::colon,
+                           tok::kw_co_return, tok::kw_co_await, tok::kw_co_yield,
                            tok::equal, tok::kw_delete, tok::kw_sizeof,
                            tok::kw_throw) ||
         PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
                            TT_UnaryOperator, TT_CastRParen))
       return TT_UnaryOperator;
 
     if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))
       return TT_PointerOrReference;

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsUnaryOperators) {
   verifyFormat("const NSPoint kBrowserFrameViewPatternOffset = {+5, -3};");
 
   verifyFormat("int a = /* confusing comment */ -1;");
   // FIXME: The space after 'i' is wrong, but hopefully, this is a rare case.
   verifyFormat("int a = i /* confusing comment */++;");
 
   verifyFormat("co_yield -1;");
   verifyFormat("co_return -1;");
+
+  // Check that * is not treated as a binary operator when we set PointerAlignment
+  // as PAS_Left after a keyword and not a declaration.
+  FormatStyle PASLeftStyle = getLLVMStyle();
+  PASLeftStyle.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("co_return *a;", PASLeftStyle);
+  verifyFormat("co_await *a;", PASLeftStyle);
+  verifyFormat("co_yield *a", PASLeftStyle);
+  verifyFormat("return *a;", PASLeftStyle);
 }
 
 TEST_F(FormatTest, DoesNotIndentRelativeToUnaryOperators) {
   verifyFormat("if (!aaaaaaaaaa( // break\n"
                "        aaaaa)) {\n"
                "}");
   verifyFormat("aaaaaaaaaa(!aaaaaaaaaa( // break\n"
                "    aaaaa));");