Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

[tberghammer]

Summary

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.

[clayborg]

Switch over to using llvm::StringRef in the functions where mentioned.

source/Symbol/Type.cpp

622–623

We should probably switch over to using "llvm::StringRef &name" here.

627

use name.empty() from llvm::StringRef

627

Remove and just use "llvm::StringRef &name" variable.

628–629

Change to use llvm::StringRef::consume_front and remove the "name_cstr += 7":

if (name.consume_front("struct ")) {
  type_class = eTypeClassStruct;
} else if (...) {

629–668

Use llvm::StringRef and its member functions instead of strstr and starchy below.

631

ditto

634

ditto

637

ditto

640

ditto

[clayborg]

Fix the consume_front mentioned in the inlined comment and this is good to go.

source/Core/Module.cpp

1010–1011

Use consume_front here:

if (type_scope.consume_front("::"))
  exact_match = true;

include/lldb/Symbol/Type.h

public:
   // Get the clang type and leave class/struct/union/enum types as forward
   // declarations if they haven't already been fully defined.
   CompilerType GetForwardCompilerType();
 
   static int Compare(const Type &a, const Type &b);
 
   // 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; }
 
   uint32_t GetEncodingMask();
 
   bool IsCompleteObjCClass() { return m_flags.is_complete_objc_class; }
 
   void SetIsCompleteObjCClass(bool is_complete_objc_class) {

source/Core/Module.cpp

 
 size_t Module::FindTypes(
     const SymbolContext &sc, const ConstString &name, bool exact_match,
     size_t max_matches,
     llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files,
     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;
   if (Type::GetTypeScopeAndBasename(type_name_cstr, type_scope, type_basename,
                                     type_class)) {
     // Check if "name" starts with "::" which means the qualified type starts
     // 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
 
     if (type_scope.size() >= 2 && type_scope[0] == ':' &&
         type_scope[1] == ':') {
-      type_scope.erase(0, 2);
+      type_scope = type_scope.drop_back(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,
                                      exact_match);
       num_matches = typesmap.GetSize();
     }
   } else {
     // The type is not in a namespace/class scope, just search for it by

source/Symbol/Type.cpp

int Type::Compare(const Type &a, const Type &b) {
   if (a_uid > b_uid)
     return 1;
   return 0;
 }
 
 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;
+  if (name.empty())
+    return false;
+
+  basename = name;
+  if (basename.consume_front("struct "))
+  else if (basename.consume_front("class "))
+  else if (basename.consume_front("union "))
+  else if (basename.consume_front("enum "))
+  else if (basename.consume_front("typedef "))
+
+  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);
     }
-    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;
-      }
-    }
+    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;
 }
 
 ModuleSP Type::GetModule() {
   if (m_symbol_file)
     return m_symbol_file->GetObjectFile()->GetModule();
   return ModuleSP();

source/Symbol/TypeList.cpp

 void TypeList::Dump(Stream *s, bool show_context) {
   for (iterator pos = m_types.begin(), end = m_types.end(); pos != end; ++pos) {
     pos->get()->Dump(s, 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);
 }
 
 void TypeList::RemoveMismatchedTypes(const std::string &type_scope,
                                      const std::string &type_basename,
                                      TypeClass type_class, bool exact_match) {
if (type_class != eTypeClassAny) {
       match_type_class = the_type->GetForwardCompilerType().GetTypeClass();
       if ((match_type_class & type_class) == 0)
         continue;
     }
 
     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)) {
         if (match_type_basename == type_basename) {
           const size_t type_scope_size = type_scope.size();
           const size_t match_type_scope_size = match_type_scope.size();
           if (exact_match || (type_scope_size == match_type_scope_size)) {
             keep_match = match_type_scope == type_scope;

source/Symbol/TypeMap.cpp

 void TypeMap::Dump(Stream *s, bool show_context) {
   for (iterator pos = m_types.begin(), end = m_types.end(); pos != end; ++pos) {
     pos->second->Dump(s, 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);
 }
 
 void TypeMap::RemoveMismatchedTypes(const std::string &type_scope,
                                     const std::string &type_basename,
                                     TypeClass type_class, bool exact_match) {
if (type_class != eTypeClassAny) {
       match_type_class = the_type->GetForwardCompilerType().GetTypeClass();
       if ((match_type_class & type_class) == 0)
         continue;
     }
 
     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)) {
         if (match_type_basename == type_basename) {
           const size_t type_scope_size = type_scope.size();
           const size_t match_type_scope_size = match_type_scope.size();
           if (exact_match || (type_scope_size == match_type_scope_size)) {
             keep_match = match_type_scope == type_scope;

unittests/Symbol/CMakeLists.txt

 add_lldb_unittest(SymbolTests
   TestClangASTContext.cpp
+  TestType.cpp
   )

unittests/Symbol/TestType.cpp

+//===-- 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>");
+}