[Lldb-commits] [lldb] 8016d61 - [LLDB] CPlusPlusNameParser does not handles templated operator< properly

[via lldb-commits]

Author: shafik
Date: 2020-03-27T14:46:39-07:00
New Revision: 8016d61e3cf4967dd28c8f6296cec685dadcaee6

URL: https://github.com/llvm/llvm-project/commit/8016d61e3cf4967dd28c8f6296cec685dadcaee6
DIFF: https://github.com/llvm/llvm-project/commit/8016d61e3cf4967dd28c8f6296cec685dadcaee6.diff

LOG: [LLDB] CPlusPlusNameParser does not handles templated operator< properly

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, because of idiosyncrasy is how clang generates debug info for these cases.

It uses 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.

Differential Revision: https://reviews.llvm.org/D76168

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
index fb85609a249d..a834be96444d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,37 @@ bool CPlusPlusNameParser::ConsumeOperator() {
   }
 
   const auto &token = Peek();
+
+  // When clang generates debug info it adds template parameters to names.
+  // Since clang doesn'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 of these cases we will not parse them correctly. 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:

diff  --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
index 4928534a40d1..e8e6a4c0da01 100644
--- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@ TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
        "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 in how clang generates 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 @@ TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
       "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) {