Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

Previously it failed to handle nested types inside templated classes
making it impossible to look up these types using the fully qualified
name.

Differential revision: https://reviews.llvm.org/D28466

llvm-svn: 291559

lldb/include/lldb/Symbol/Type.h

@@ -201,8 +201,9 @@ class Type : public std::enable_shared_from_this<Type>, public UserID {
 
   // From a fully qualified typename, split the type into the type basename
   // and the remaining type scope (namespaces/classes).
-  static bool GetTypeScopeAndBasename(const char *&name_cstr,
-                                      std::string &scope, std::string &basename,
+  static bool GetTypeScopeAndBasename(const llvm::StringRef& name,
+                                      llvm::StringRef &scope,
+                                      llvm::StringRef &basename,
                                       lldb::TypeClass &type_class);
   void SetEncodingType(Type *encoding_type) { m_encoding_type = encoding_type; }
 

lldb/source/Core/Module.cpp

@@ -995,8 +995,8 @@ size_t Module::FindTypes(
     TypeList &types) {
   size_t num_matches = 0;
   const char *type_name_cstr = name.GetCString();
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   const bool append = true;
   TypeClass type_class = eTypeClassAny;
   TypeMap typesmap;
@@ -1006,13 +1006,9 @@ size_t Module::FindTypes(
     // from the root namespace and implies and exact match. The typenames we
     // get back from clang do not start with "::" so we need to strip this off
     // in order to get the qualified names to match
+    exact_match = type_scope.consume_front("::");
 
-    if (type_scope.size() >= 2 && type_scope[0] == ':' &&
-        type_scope[1] == ':') {
-      type_scope.erase(0, 2);
-      exact_match = true;
-    }
-    ConstString type_basename_const_str(type_basename.c_str());
+    ConstString type_basename_const_str(type_basename);
     if (FindTypes_Impl(sc, type_basename_const_str, nullptr, append,
                        max_matches, searched_symbol_files, typesmap)) {
       typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,

lldb/source/Symbol/Type.cpp

@@ -620,50 +620,59 @@ ConstString Type::GetQualifiedName() {
   return GetForwardCompilerType().GetConstTypeName();
 }
 
-bool Type::GetTypeScopeAndBasename(const char *&name_cstr, std::string &scope,
-                                   std::string &basename,
+bool Type::GetTypeScopeAndBasename(const llvm::StringRef& name,
+                                   llvm::StringRef &scope,
+                                   llvm::StringRef &basename,
                                    TypeClass &type_class) {
-  // Protect against null c string.
-
   type_class = eTypeClassAny;
 
-  if (name_cstr && name_cstr[0]) {
-    llvm::StringRef name_strref(name_cstr);
-    if (name_strref.startswith("struct ")) {
-      name_cstr += 7;
-      type_class = eTypeClassStruct;
-    } else if (name_strref.startswith("class ")) {
-      name_cstr += 6;
-      type_class = eTypeClassClass;
-    } else if (name_strref.startswith("union ")) {
-      name_cstr += 6;
-      type_class = eTypeClassUnion;
-    } else if (name_strref.startswith("enum ")) {
-      name_cstr += 5;
-      type_class = eTypeClassEnumeration;
-    } else if (name_strref.startswith("typedef ")) {
-      name_cstr += 8;
-      type_class = eTypeClassTypedef;
-    }
-    const char *basename_cstr = name_cstr;
-    const char *namespace_separator = ::strstr(basename_cstr, "::");
-    if (namespace_separator) {
-      const char *template_arg_char = ::strchr(basename_cstr, '<');
-      while (namespace_separator != nullptr) {
-        if (template_arg_char &&
-            namespace_separator > template_arg_char) // but namespace'd template
-                                                     // arguments are still good
-                                                     // to go
-          break;
-        basename_cstr = namespace_separator + 2;
-        namespace_separator = strstr(basename_cstr, "::");
-      }
-      if (basename_cstr > name_cstr) {
-        scope.assign(name_cstr, basename_cstr - name_cstr);
-        basename.assign(basename_cstr);
-        return true;
+  if (name.empty())
+    return false;
+
+  basename = name;
+  if (basename.consume_front("struct "))
+    type_class = eTypeClassStruct;
+  else if (basename.consume_front("class "))
+    type_class = eTypeClassClass;
+  else if (basename.consume_front("union "))
+    type_class = eTypeClassUnion;
+  else if (basename.consume_front("enum "))
+    type_class = eTypeClassEnumeration;
+  else if (basename.consume_front("typedef "))
+    type_class = eTypeClassTypedef;
+
+  size_t namespace_separator = basename.find("::");
+  if (namespace_separator == llvm::StringRef::npos)
+    return false;
+
+  size_t template_begin = basename.find('<');
+  while (namespace_separator != llvm::StringRef::npos) {
+    if (template_begin != llvm::StringRef::npos &&
+        namespace_separator > template_begin) {
+      size_t template_depth = 1;
+      llvm::StringRef template_arg =
+          basename.drop_front(template_begin + 1);
+      while (template_depth > 0 && !template_arg.empty()) {
+        if (template_arg.front() == '<')
+          template_depth++;
+        else if (template_arg.front() == '>')
+          template_depth--;
+        template_arg = template_arg.drop_front(1);
       }
+      if (template_depth != 0)
+        return false; // We have an invalid type name. Bail out.
+      if (template_arg.empty())
+        break; // The template ends at the end of the full name.
+      basename = template_arg;
+    } else {
+      basename = basename.drop_front(namespace_separator + 2);
     }
+    template_begin = basename.find('<');
+    namespace_separator = basename.find("::");
+  }
+  if (basename.size() < name.size()) {
+    scope = name.take_front(name.size() - basename.size());
+    return true;
   }
   return false;
 }

lldb/source/Symbol/TypeList.cpp

@@ -108,13 +108,13 @@ void TypeList::Dump(Stream *s, bool show_context) {
 
 void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
                                      bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
                                      type_basename, type_class)) {
     type_basename = qualified_typename;
-    type_scope.clear();
+    type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,
                                exact_match);
@@ -145,8 +145,8 @@ void TypeList::RemoveMismatchedTypes(const std::string &type_scope,
     ConstString match_type_name_const_str(the_type->GetQualifiedName());
     if (match_type_name_const_str) {
       const char *match_type_name = match_type_name_const_str.GetCString();
-      std::string match_type_scope;
-      std::string match_type_basename;
+      llvm::StringRef match_type_scope;
+      llvm::StringRef match_type_basename;
       if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
                                         match_type_basename,
                                         match_type_class)) {

lldb/source/Symbol/TypeMap.cpp

@@ -152,13 +152,13 @@ void TypeMap::Dump(Stream *s, bool show_context) {
 
 void TypeMap::RemoveMismatchedTypes(const char *qualified_typename,
                                     bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
                                      type_basename, type_class)) {
     type_basename = qualified_typename;
-    type_scope.clear();
+    type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,
                                exact_match);
@@ -189,8 +189,8 @@ void TypeMap::RemoveMismatchedTypes(const std::string &type_scope,
     ConstString match_type_name_const_str(the_type->GetQualifiedName());
     if (match_type_name_const_str) {
       const char *match_type_name = match_type_name_const_str.GetCString();
-      std::string match_type_scope;
-      std::string match_type_basename;
+      llvm::StringRef match_type_scope;
+      llvm::StringRef match_type_basename;
       if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
                                         match_type_basename,
                                         match_type_class)) {

lldb/unittests/Symbol/CMakeLists.txt

@@ -1,3 +1,4 @@
 add_lldb_unittest(SymbolTests
   TestClangASTContext.cpp
+  TestType.cpp
   )

lldb/unittests/Symbol/TestType.cpp

@@ -0,0 +1,51 @@
+//===-- TestType.cpp --------------------------------------------*- C++ -*-===//
+//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Symbol/Type.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+void TestGetTypeScopeAndBasenameHelper(const char *full_type,
+                                       bool expected_is_scoped,
+                                       const char *expected_scope,
+                                       const char *expected_name) {
+  llvm::StringRef scope, name;
+  lldb::TypeClass type_class;
+  bool is_scoped =
+      Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+  EXPECT_EQ(is_scoped, expected_is_scoped);
+  if (expected_is_scoped) {
+    EXPECT_EQ(scope, expected_scope);
+    EXPECT_EQ(name, expected_name);
+  }
+}
+};
+
+TEST(Type, GetTypeScopeAndBasename) {
+  TestGetTypeScopeAndBasenameHelper("int", false, "", "");
+  TestGetTypeScopeAndBasenameHelper("std::string", true, "std::", "string");
+  TestGetTypeScopeAndBasenameHelper("std::set<int>", true, "std::", "set<int>");
+  TestGetTypeScopeAndBasenameHelper("std::set<int, std::less<int>>", true,
+                                    "std::", "set<int, std::less<int>>");
+  TestGetTypeScopeAndBasenameHelper("std::string::iterator", true,
+                                    "std::string::", "iterator");
+  TestGetTypeScopeAndBasenameHelper("std::set<int>::iterator", true,
+                                    "std::set<int>::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+      "std::set<int, std::less<int>>::iterator", true,
+      "std::set<int, std::less<int>>::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+      "std::set<int, std::less<int>>::iterator<bool>", true,
+      "std::set<int, std::less<int>>::", "iterator<bool>");
+}