[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

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



================
Comment at: source/Symbol/Type.cpp:623
 
 bool Type::GetTypeScopeAndBasename(const char *&name_cstr, std::string &scope,
                                    std::string &basename,
----------------
We should probably switch over to using "llvm::StringRef &name" here.


================
Comment at: source/Symbol/Type.cpp:629
   // Protect against null c string.
+  if (!name_cstr || !name_cstr[0])
+    return false;
----------------
use name.empty() from llvm::StringRef


================
Comment at: source/Symbol/Type.cpp:632
 
-  type_class = eTypeClassAny;
+  llvm::StringRef name_strref(name_cstr);
+  if (name_strref.startswith("struct ")) {
----------------
Remove and just use "llvm::StringRef &name" variable.


================
Comment at: source/Symbol/Type.cpp:633-634
+  llvm::StringRef name_strref(name_cstr);
+  if (name_strref.startswith("struct ")) {
+    name_cstr += 7;
+    type_class = eTypeClassStruct;
----------------
Change to use llvm::StringRef::consume_front and remove the "name_cstr += 7":

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


================
Comment at: source/Symbol/Type.cpp:636
+    type_class = eTypeClassStruct;
+  } else if (name_strref.startswith("class ")) {
+    name_cstr += 6;
----------------
ditto


================
Comment at: source/Symbol/Type.cpp:639
+    type_class = eTypeClassClass;
+  } else if (name_strref.startswith("union ")) {
+    name_cstr += 6;
----------------
ditto


================
Comment at: source/Symbol/Type.cpp:642
+    type_class = eTypeClassUnion;
+  } else if (name_strref.startswith("enum ")) {
+    name_cstr += 5;
----------------
ditto


================
Comment at: source/Symbol/Type.cpp:645
+    type_class = eTypeClassEnumeration;
+  } else if (name_strref.startswith("typedef ")) {
+    name_cstr += 8;
----------------
ditto


================
Comment at: source/Symbol/Type.cpp:650-651
 
-  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;
+  const char *basename_cstr = name_cstr;
+  const char *namespace_separator = ::strstr(basename_cstr, "::");
+  const char *template_arg_char = ::strchr(basename_cstr, '<');
----------------
Use llvm::StringRef and its member functions instead of strstr and starchy below.


https://reviews.llvm.org/D28466