[clang-format] Correctly annotate operator free function call

[rymiel]

Summary

The annotator correctly annotates an overloaded operator call when
called as a member function, like x.operator+(y), however, when called
as a free function, like operator+(x, y), the annotator assumed it was
an overloaded operator function *declaration*, instead of a call.

This patch allows for a free function call to correctly be annotated as
a call, but only if the current like cannot be a declaration, usually
within the bodies of a function.

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

[Nuullll]

Thanks, LGTM!

clang/lib/Format/TokenAnnotator.cpp

312–313

minor. Probably better to rename it to something like IsOperatorCallSite.

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

312–313

The Name should really be changes, since it's not limited to member functions anymore.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

314

Why not Line.InMacroBody? Wouldn't it misformat the following snippet?

#define FOO           \
  void foo() {        \
    operator+(a * b); \
  }

[rymiel]

clang/lib/Format/TokenAnnotator.cpp

314

Yes, but it would break this test case: https://github.com/llvm/llvm-project/blob/e469d0d636f36140b08d0b5f603c043008307bcf/clang/unittests/Format/FormatTest.cpp#L11573

I understand it's a nasty workaround, though, but it's quick. The other option seems to be rewriting how overloaded operators are annotated to instead be more like regular function declarations, but I haven't gauged how hard that would be

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

314

Yeah. Can you add the snippet as a FIXME test case (under #if 0)?

[rymiel]

Address comments

clang/lib/Format/TokenAnnotator.cpp

if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
       FormatToken *Prev = &OpeningParen;
       while (!Prev->is(tok::kw_operator)) {
         Prev = Prev->Previous;
         assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
       }
 
       // If faced with "a.operator*(argument)" or "a->operator*(argument)",
       // i.e. the operator is called as a member function,
       // then the argument must be an expression.
-      bool OperatorCalledAsMemberFunction =
-          Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
-      Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
+      // If faced with "operator+(argument)", i.e. the operator is called as
+      // a free function, then the argument is an expression only if the current
+      // line can't be a declaration.
+      bool IsOperatorCallSite =
+          (Prev->Previous &&
+           Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+          (!Line.MustBeDeclaration && !Line.InMacroBody);
+      Contexts.back().IsExpression = IsOperatorCallSite;
     } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
       Contexts.back().IsExpression = true;
       Contexts.back().ContextType = Context::VerilogInstancePortList;
     } else if (Style.isJavaScript() &&
                (Line.startsWith(Keywords.kw_type, tok::identifier) ||
                 Line.startsWith(tok::kw_export, Keywords.kw_type,
                                 tok::identifier))) {
       // type X = (...);

clang/unittests/Format/FormatTest.cpp

EXPECT_EQ("#define OP(x)                                    \\\n"
             "    return s << a.DebugString();                 \\\n"
             "  }",
             format("#define OP(x) \\\n"
                    "  ostream &operator<<(ostream &s, const A &a) { \\\n"
                    "    return s << a.DebugString(); \\\n"
                    "  }",
                    getLLVMStyleWithColumns(50)));
 
+// FIXME: We should be able to figure out this is an operator call
+#if 0
+  verifyFormat("#define FOO             \\\n"
+               "  void foo() {          \\\n"
+               "    operator+(a * b);   \\\n"
+               "  }", getLLVMStyleWithColumns(25));
+#endif
+
   // FIXME: We cannot handle this case yet; we might be able to figure out that
   // foo<x> d > v; doesn't make sense.
   verifyFormat("foo<a<b && c> d> v;");
 
   FormatStyle PointerMiddle = getLLVMStyle();
   PointerMiddle.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("delete *x;", PointerMiddle);
   verifyFormat("int * x;", PointerMiddle);

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsOverloadedOperators) {
   EXPECT_TOKEN(Tokens[2], tok::equal, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_OverloadedOperatorLParen);
   Tokens = annotate("auto operator()() {}");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::kw_operator, TT_FunctionDeclarationName);
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+                    "  int operator+(a* b);\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+                    "  operator+(a * b);\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+                    "  void foo() {\n"
+                    "    operator+(a * b);\n"
+                    "  }\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
   struct {
     const char *Text;
     tok::TokenKind Kind;
   } Operators[] = {{"+", tok::plus},
                    {"-", tok::minus},