shafik created this revision.
shafik added reviewers: aprantl, labath, jingham.
shafik added a comment.

My fix in `ConsumeOperator()` is not proper but if everyone feels this is 
correct approach I will create member functions to deal with this cleanly.

Other approaches could be modifying `ExtractTokens()` to detect this case and 
generate two `tok::less`in place of `tok::lessless` but this feels like the 
wrong place to fix this. I tried to understand how clang handles this case 
since but it was not obvious. AFAICT they have to deal with this case too.


CPlusPlusNameParser is used in several places on of them is during IR execution 
and setting breakpoints to pull information C++ like the basename, the context 
and arguments.

Currently it does not handle templated `operator<` properly. It used 
`clang::Lexer` which will tokenize `operator<<A::B>` into:

tok::kw_operator
tok::lessless
tok::raw_identifier

Later on the parser in `ConsumeOperator()` does not handle this case properly 
and we end up failing to parse.


https://reviews.llvm.org/D76168

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp


Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
        "std::vector<Class, std::allocator<Class>>",
        "_M_emplace_back_aux<Class const&>"},
       {"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
-      {"`operator<<A>'::`2'::B<0>::operator>",
-       "`operator<<A>'::`2'::B<0>",
+      {"`operator<<A>'::`2'::B<0>::operator>", "`operator<<A>'::`2'::B<0>",
        "operator>"},
       {"`anonymous namespace'::S::<<::__l2::Foo",
-       "`anonymous namespace'::S::<<::__l2",
-       "Foo"}};
+       "`anonymous namespace'::S::<<::__l2", "Foo"},
+      {"A::operator><A::B>", "A", "operator><A::B>"},
+      {"operator><A::B>", "", "operator><A::B>"},
+      {"A::operator<<A::B>", "A", "operator<<A::B>"},
+      {"operator<<A::B>", "", "operator<<A::B>"},
+      {"A::operator<<<A::B>", "A", "operator<<<A::B>"},
+      {"operator<<<A::B>", "", "operator<<<A::B>"},
+      // We expect these cases to fail until we turn on C++2a
+      //      {"A::operator<=><A::B>", "A", "operator<=><A::B>"},
+      //      {"operator<=><A::B>", "", "operator<=><A::B>" },
+  };
 
   llvm::StringRef context, basename;
   for (const auto &test : test_cases) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,24 @@
   }
 
   const auto &token = Peek();
+  if (token.getKind() == tok::lessless) {
+    if (m_next_token_index + 1 < m_tokens.size()) {
+      clang::Token n_token = m_tokens[m_next_token_index + 1];
+      if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less) 
{
+        clang::Token tmp_tok;
+
+        tmp_tok.setLength(1);
+        tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
+        tmp_tok.setKind(tok::less);
+
+        m_tokens[m_next_token_index] = tmp_tok;
+
+        start_position.Remove();
+        return true;
+      }
+    }
+  }
+
   switch (token.getKind()) {
   case tok::kw_new:
   case tok::kw_delete:


Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
        "std::vector<Class, std::allocator<Class>>",
        "_M_emplace_back_aux<Class const&>"},
       {"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
-      {"`operator<<A>'::`2'::B<0>::operator>",
-       "`operator<<A>'::`2'::B<0>",
+      {"`operator<<A>'::`2'::B<0>::operator>", "`operator<<A>'::`2'::B<0>",
        "operator>"},
       {"`anonymous namespace'::S::<<::__l2::Foo",
-       "`anonymous namespace'::S::<<::__l2",
-       "Foo"}};
+       "`anonymous namespace'::S::<<::__l2", "Foo"},
+      {"A::operator><A::B>", "A", "operator><A::B>"},
+      {"operator><A::B>", "", "operator><A::B>"},
+      {"A::operator<<A::B>", "A", "operator<<A::B>"},
+      {"operator<<A::B>", "", "operator<<A::B>"},
+      {"A::operator<<<A::B>", "A", "operator<<<A::B>"},
+      {"operator<<<A::B>", "", "operator<<<A::B>"},
+      // We expect these cases to fail until we turn on C++2a
+      //      {"A::operator<=><A::B>", "A", "operator<=><A::B>"},
+      //      {"operator<=><A::B>", "", "operator<=><A::B>" },
+  };
 
   llvm::StringRef context, basename;
   for (const auto &test : test_cases) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,24 @@
   }
 
   const auto &token = Peek();
+  if (token.getKind() == tok::lessless) {
+    if (m_next_token_index + 1 < m_tokens.size()) {
+      clang::Token n_token = m_tokens[m_next_token_index + 1];
+      if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less) {
+        clang::Token tmp_tok;
+
+        tmp_tok.setLength(1);
+        tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
+        tmp_tok.setKind(tok::less);
+
+        m_tokens[m_next_token_index] = tmp_tok;
+
+        start_position.Remove();
+        return true;
+      }
+    }
+  }
+
   switch (token.getKind()) {
   case tok::kw_new:
   case tok::kw_delete:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to