Check for lack of C++ context first when demangling

[scott.smith]

Summary

It seems that if we have no context, then it can't possibly be a method. Check that first.

[scott.smith]

FYI, moving this code changes how two symbols (out of 2M+) are handled on a test program I have. I noticed this because when I changed the loop to set it up for parallelization, I effectively deferred all handling of C++ names to later. It simplifies the code to handle !const_context situations first, and then handle the rest. At a high level I assume that should be equivalent, but it actually changed two symbols.

I send this review out so you can either:

1. Tell me it's wrong to move the if statement, because of some situation I don't understand, or
2. Tell me this is correct, and the two symbols that are handled differently actually should be handled differently, or
3. Tell me this is correct, and the two symbols that are handled differently are due to a bug somewhere else.

FWIW the two symbols demangle to roughly "thing<lots_of_stuff>". lots_of_stuff includes lambdas, vectors, and some other things. It wouldn't surprise me if I stumbled across a bug in the mangler or demangler (there are lots of repeated sections, so it could be a bug in back references).

[scott.smith]

Can someone please commit this? Thank you!

[labath]

I am having trouble applying this. Do you need to rebase or something?

[scott.smith]

In D32708#743161, @labath wrote:

I am having trouble applying this. Do you need to rebase or something?

It was based on another commit that you committed for me, but committed after trying to commit this one. It should apply now that you committed D32316.

source/Symbol/Symtab.cpp

for (entry.value = 0; entry.value < num_symbols; ++entry.value) {
             if (entry.cstring) {
               // ConstString objects permanently store the string in the pool so
               // calling
               // GetCString() on the value gets us a const char * that will
               // never go away
               const char *const_context =
                   ConstString(cxx_method.GetContext()).GetCString();
 
+              if (!const_context || const_context[0] == 0) {
+                // No context for this function so this has to be a basename
+                m_basename_to_index.Append(entry);
+                // If there is no context (no namespaces or class scopes that
+                // come before the function name) then this also could be a
+                // fullname.
+                m_name_to_index.Append(entry);
+              } else {
                 entry_ref = entry.cstring.GetStringRef();
                 if (entry_ref[0] == '~' ||
                     !cxx_method.GetQualifiers().empty()) {
                   // The first character of the demangled basename is '~' which
                   // means we have a class destructor. We can use this information
                   // to help us know what is a class and what isn't.
                   if (class_contexts.find(const_context) == class_contexts.end())
                     class_contexts.insert(const_context);
                   m_method_to_index.Append(entry);
                 } else {
-                if (const_context && const_context[0]) {
                   if (class_contexts.find(const_context) !=
                       class_contexts.end()) {
                     // The current decl context is in our "class_contexts" which
                     // means
                     // this is a method on a class
                     m_method_to_index.Append(entry);
                   } else {
                     // We don't know if this is a function basename or a method,
                     // so put it into a temporary collection so once we are done
                     // we can look in class_contexts to see if each entry is a
                     // class
                     // or just a function and will put any remaining items into
                     // m_method_to_index or m_basename_to_index as needed
                     mangled_name_to_index.Append(entry);
                     symbol_contexts[entry.value] = const_context;
                   }
-                } else {
-                  // No context for this function so this has to be a basename
-                  m_basename_to_index.Append(entry);
-                  // If there is no context (no namespaces or class scopes that
-                  // come before the function name) then this also could be a
-                  // fullname.
-                  if (cxx_method.GetContext().empty())
-                    m_name_to_index.Append(entry);
                 }
               }
             }
           }
         }
       }
 
       entry.cstring = mangled.GetDemangledName(symbol->GetLanguage());