[Lldb-commits] [PATCH] D62756: Be consistent when adding names and mangled names to the index

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, aprantl, JDevlieghere, jingham, jasonmolenda.
Herald added a subscriber: arphaman.

We were being very inconsistent with the way we were adding names to the indexes.

For DW_TAG_subprogram and DW_TAG_inlined_subroutine we would not add the mangled name if the we didn't have a simple name. We would only add the mangled name if started with '_' (even if it matches the simple name that started with '_') or if the name differed if the mangled name didn't start with '_'.

For DW_TAG_array_type, DW_TAG_base_type, DW_TAG_class_type, DW_TAG_constant, DW_TAG_enumeration_type, DW_TAG_string_type, DW_TAG_structure_type, DW_TAG_subroutine_type, DW_TAG_typedef, DW_TAG_union_type, and DW_TAG_unspecified_type we would add the simple name and the mangled name if either existed, even if they were the same..

For DW_TAG_variable we would only follow the same rule as the DW_TAG_subprogram and DW_TAG_inlined_subroutine (so no simple name meant we wouldn't add the mangled name).

Not sure if this was on purpose or not, but wanted to post this diff for discussion on what the right thing to do should be.


https://reviews.llvm.org/D62756

Files:
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -110,6 +110,16 @@
   }
 }
 
+static inline bool AddMangled(const char *name, const char *mangled) {
+  if (mangled == nullptr || name == mangled)
+    return false;
+  if (name == nullptr)
+    return true;
+  if (mangled[0] != name[0])
+    return true;
+  return ::strcmp(name, mangled) != 0;
+}
+
 void ManualDWARFIndex::IndexUnitImpl(
     DWARFUnit &unit, const LanguageType cu_language,
     const dw_offset_t cu_offset, IndexSet &set) {
@@ -281,17 +291,9 @@
           if (!is_method && !mangled_cstr && !objc_method.IsValid(true))
             set.function_fullnames.Insert(ConstString(name), ref);
         }
-        if (mangled_cstr) {
-          // Make sure our mangled name isn't the same string table entry as
-          // our name. If it starts with '_', then it is ok, else compare the
-          // string to make sure it isn't the same and we don't end up with
-          // duplicate entries
-          if (name && name != mangled_cstr &&
-              ((mangled_cstr[0] == '_') ||
-               (::strcmp(name, mangled_cstr) != 0))) {
-            set.function_fullnames.Insert(ConstString(mangled_cstr), ref);
-          }
-        }
+        // Make sure our mangled name isn't the same as our name.
+        if (AddMangled(name, mangled_cstr))
+         set.function_fullnames.Insert(ConstString(mangled_cstr), ref);
       }
       break;
 
@@ -306,10 +308,13 @@
     case DW_TAG_typedef:
     case DW_TAG_union_type:
     case DW_TAG_unspecified_type:
-      if (name && !is_declaration)
-        set.types.Insert(ConstString(name), ref);
-      if (mangled_cstr && !is_declaration)
-        set.types.Insert(ConstString(mangled_cstr), ref);
+      if (!is_declaration) {
+        if (name)
+          set.types.Insert(ConstString(name), ref);
+        // Make sure our mangled name isn't the same as our name.
+        if (AddMangled(name, mangled_cstr))
+          set.types.Insert(ConstString(mangled_cstr), ref);
+      }
       break;
 
     case DW_TAG_namespace:
@@ -318,21 +323,17 @@
       break;
 
     case DW_TAG_variable:
-      if (name && has_location_or_const_value && is_global_or_static_variable) {
-        set.globals.Insert(ConstString(name), ref);
+      if (has_location_or_const_value && is_global_or_static_variable) {
+        if (name)
+          set.globals.Insert(ConstString(name), ref);
         // Be sure to include variables by their mangled and demangled names if
         // they have any since a variable can have a basename "i", a mangled
         // named "_ZN12_GLOBAL__N_11iE" and a demangled mangled name
         // "(anonymous namespace)::i"...
 
-        // Make sure our mangled name isn't the same string table entry as our
-        // name. If it starts with '_', then it is ok, else compare the string
-        // to make sure it isn't the same and we don't end up with duplicate
-        // entries
-        if (mangled_cstr && name != mangled_cstr &&
-            ((mangled_cstr[0] == '_') || (::strcmp(name, mangled_cstr) != 0))) {
+        // Make sure our mangled name isn't the same as our name.
+        if (AddMangled(name, mangled_cstr))
           set.globals.Insert(ConstString(mangled_cstr), ref);
-        }
       }
       break;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62756.202495.patch
Type: text/x-patch
Size: 3470 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190531/387b3b25/attachment.bin>