[Lldb-commits] [PATCH] D76168: CPlusPlusNameParser does not handles templated operator< properly

[Shafik Yaghmour via Phabricator via lldb-commits]

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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76168.250326.patch
Type: text/x-patch
Size: 2362 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200314/8ecd0c66/attachment-0001.bin>