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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 252941.
shafik marked 4 inline comments as done.
shafik added a comment.

Addressing comments:

- Adding more detailed comments
- Adding test for cases that currently fail b/c we don't enable C++20


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76168/new/

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"},
+      // These cases are idiosyncratic to how we generate debug info for names
+      // when we have template parameters. They are not valid C++ names but if
+      // we fix this we need to support them for older compilers.
+      {"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>"},
+  };
 
   llvm::StringRef context, basename;
   for (const auto &test : test_cases) {
@@ -169,6 +177,12 @@
       "abc::", context, basename));
   EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
       "f<A<B><C>>", context, basename));
+
+  // We expect these cases to fail until we turn on C++2a
+  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+      "A::operator<=><A::B>", context, basename));
+  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+      "operator<=><A::B>", context, basename));
 }
 
 static std::set<std::string> FindAlternate(llvm::StringRef Name) {
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,37 @@
   }
 
   const auto &token = Peek();
+
+  // When we generate debug info we add template parameters to names.
+  // Since we don't add a space between the name and the template parameter in
+  // some cases we are not generating valid C++ names e.g.:
+  //
+  //   operator<<A::B>
+  //
+  // In some cases we with then parse incorrectly. This fixes the issue by
+  // detecting this case and inserting tok::less in place of tok::lessless
+  // and returning successfully that we consumed the operator.
+  if (token.getKind() == tok::lessless) {
+    // Make sure we have more tokens before attempting to look ahead one more
+    if (m_next_token_index + 1 < m_tokens.size()) {
+      // Look ahead two tokens
+      clang::Token n_token = m_tokens[m_next_token_index + 1];
+      // If we find ( or < then this is indeed operator<< no need for fix
+      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.252941.patch
Type: text/x-patch
Size: 3574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200326/80852703/attachment.bin>