[clang-format] Fix bug in parsing `operator<` with template

[pjessesco]

Summary

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

This patch handles a bug when parsing a below example code :

template <class> class S;

template <class T> bool operator<(S<T> const &x, S<T> const &y) {
  return x.i < y.i;
}

template <class T> class S {
  int i = 42;
  friend bool operator< <>(S const &, S const &);
};

int main() { return S<int>{} < S<int>{}; }

which parse < <> as << >, not < <> in terms of tokens as discussed in discord.

1. Add a condition in tryMergeLessLess() considering operator keyword and >
2. Force to leave a whitespace between tok::less and a template opener
3. Add unit test

[pjessesco]

Add deleted newline in FormatTest.cpp by mistake

[pjessesco]

Fix to pass unittest

[MyDeveloperDay]

clang/lib/Format/FormatTokenLexer.cpp

432

isn't this going to crash if Tokens.size() is 3?

[curdeius]

clang/lib/Format/FormatTokenLexer.cpp

432

It probably will. Anyway, it's a UB. Please do it differently. Also, please fix the typo: "Fourth".

clang/unittests/Format/FormatTest.cpp

9278

Please add a test for a possible crash: something like verifyFormat("< <>");

[pjessesco]

Fix to accept feedbacks.

1. Forth -> Fourth
2. Add unit test verifyFormat("< <>");
3. Fix to avoid undefined behavior

[curdeius]

LGTM.
Thanks for working on this!

Do you have commit rights or do you need that someone lands it for you?
If you need help, please indicate your name and email to be used for the commit.

Also, if you think contributing more patches, have a look at https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

clang/lib/Format/FormatTokenLexer.cpp

433–440

I hate the naming here.
Unless I'm mistaken we have: Fourth, First + 0, First + 1, First + 2.
But I'll clean it up at some time.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

9380

just make sure there isn't a whitespace change here you didn't mean

9467

you doing something in your editor...

[MyDeveloperDay]

run git clang-format before submitting (if making the patch from staged files)

[owenpan]

clang/lib/Format/FormatTokenLexer.cpp

433–440

Me too. Maybe changing First to point at Fourth and get rid of Fourth, which has a different level of indirection than First now. I would probably get rid of FourthTokenIsLess as well.

[pjessesco]

Fix revision after running git clang-format.

Sorry for annoying, I thought I did not make any changes at that line. :(

name : Jino Park
email : [e-mail]

[curdeius]

clang/unittests/Format/FormatTest.cpp

9278

Looking at other related bug reports, I think that we should test operator<< <>(); as well. Could you add it please?

[curdeius]

Or I'll add it when landing.

clang/lib/Format/FormatTokenLexer.cpp

bool FormatTokenLexer::tryTransformTryUsageForC() {
   return true;
 }
 
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
   if (Tokens.size() < 3)
     return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-    FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+    FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
       First[0]->isNot(tok::less) || FourthTokenIsLess)
     return false;
 
   // Only merge if there currently is no whitespace between the two "<".
   if (First[1]->WhitespaceRange.getBegin() !=
       First[1]->WhitespaceRange.getEnd())
     return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+    return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;
   Tokens.erase(Tokens.end() - 2);
   return true;
 }
 
 bool FormatTokenLexer::tryMergeTokens(ArrayRef<tok::TokenKind> Kinds,

clang/lib/Format/TokenAnnotator.cpp

if (Left.is(tok::ellipsis) && Right.is(tok::identifier) &&
         Line.First->is(Keywords.kw_import))
       return false;
 
     if (Left.is(tok::kw_operator))
       return Right.is(tok::coloncolon);
     if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
         !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
       return true;
+    if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
+        Right.is(TT_TemplateOpener))
+      return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
              Style.Language == FormatStyle::LK_TextProto) {
     if (Right.is(tok::period) &&
         Left.isOneOf(Keywords.kw_optional, Keywords.kw_required,
                      Keywords.kw_repeated, Keywords.kw_extend))
       return true;
     if (Right.is(tok::l_paren) &&
         Left.isOneOf(Keywords.kw_returns, Keywords.kw_option))

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsOverloadedOperators) {
   verifyFormat("bool operator()();");
   verifyFormat("bool operator[]();");
   verifyFormat("operator bool();");
   verifyFormat("operator int();");
   verifyFormat("operator void *();");
   verifyFormat("operator SomeType<int>();");
   verifyFormat("operator SomeType<int, int>();");
   verifyFormat("operator SomeType<SomeType<int>>();");
+  verifyFormat("operator< <>();");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
   verifyFormat("void operator delete[](void *ptr);");
   verifyFormat("template <typename AAAAAAA, typename BBBBBBB>\n"
                "AAAAAAA operator/(const AAAAAAA &a, BBBBBBB &b);");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaa operator,(\n"
                "    aaaaaaaaaaaaaaaaaaaaa &aaaaaaaaaaaaaaaaaaaaaaaaaa) const;");
TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   BreakTemplate.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
 
   verifyFormat("struct f {\n"
                "  template <class T>\n"
                "  int &foo(const std::string &str) &noexcept {}\n"
                "};",
                BreakTemplate);
 
   verifyFormat("struct f {\n"
                "  template <class T>\n"
                "  int &foo(const std::string &str) &&noexcept {}\n"
                "};",
                BreakTemplate);
 
   verifyFormat("struct f {\n"
                "  template <class T>\n"
                "  int &foo(const std::string &str) const &noexcept {}\n"
TEST_F(FormatTest, UnderstandsNewAndDelete) {
   verifyFormat("void f() {\n"
                "  A *a = new A;\n"
                "  A *a = new (placement) A;\n"
                "  delete a;\n"
                "  delete (A *)a;\n"
                "}");
   verifyFormat("new (aaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaa))\n"
                "    typename aaaaaaaaaaaaaaaaaaaaaaaa();");
   verifyFormat("auto aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
                "    new (aaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaa))\n"
                "        typename aaaaaaaaaaaaaaaaaaaaaaaa();");
   verifyFormat("delete[] h->p;");
 
   verifyFormat("void operator delete(void *foo) ATTRIB;");
   verifyFormat("void operator new(void *foo) ATTRIB;");
   verifyFormat("void operator delete[](void *foo) ATTRIB;");
   verifyFormat("void operator delete(void *ptr) noexcept;");