[Lldb-commits] [PATCH] Patch for LLDB demangler for demangling upon actual language

[Zachary Turner]

REPOSITORY
  rL LLVM

================
Comment at: include/lldb/Core/Mangled.h:94
@@ +93,3 @@
+    explicit
+    Mangled (const ConstString &name, bool is_mangled, lldb::LanguageType language);
+
----------------
I don't like having this second argument here.  If you know in advance the name is demangled, why even use this class for anything?

================
Comment at: include/lldb/Core/Mangled.h:347
@@ -318,1 +346,3 @@
+            Language m_language;        ///< Language, used for taking into account language differences in demangling
+    mutable bool m_bLanguage_was_set;   ///< Flag, means known language was set explicitly
 };
----------------
Just call it m_language_was_set.  We don't use hungarian notation.

================
Comment at: source/Core/Mangled.cpp:5095
@@ +5094,3 @@
+//----------------------------------------------------------------------
+Mangled::Mangled (const ConstString &s, bool mangled, lldb::LanguageType language) :
+    m_mangled(),
----------------
I think the default constructor needs to be updated to initialialize m_language and m_b_language_was_set.

================
Comment at: source/Core/Mangled.cpp:5226
@@ -5211,1 +5225,3 @@
+    if ((m_mangled && !m_demangled)
+        || m_bLanguage_was_set)
     {
----------------
Instead of having this check, can you just clear m_demangled when you call SetLanguage()?  In fact, we could probably get rid of m_bLanguage_was_set entirely if you do this.

================
Comment at: source/Core/Mangled.cpp:5271-5273
@@ -5253,2 +5270,5 @@
                 {
-                    m_demangled.SetCStringWithMangledCounterpart(demangled_name, m_mangled);
+                    std::string temp_demangled_name(demangled_name);
+                    std::string cplusplus_sep("::");
+                    std::string pascaljava_sep(".");
+
----------------
can the two separators be const char * instead of an std::string (to avoid a heap allocation), and also moved inside of the case statement for Pascal / Java?  None of the other branches use them.  Also temp_demangled_name can be moved into the case as well, and the default case statement can just use demangled_name.c_str()

================
Comment at: source/Core/Mangled.cpp:5301
@@ -5265,3 +5300,3 @@
     }
-
+    m_bLanguage_was_set = false;
     return m_demangled;
----------------
This line also goes away if we delete this variable and call m_demangled.Clear() in SetLanguage()

================
Comment at: source/Core/Mangled.cpp:5364
@@ +5363,3 @@
+    if (m_language.GetLanguage() != lldb::eLanguageTypeUnknown)
+        m_bLanguage_was_set = true;
+}
----------------
Should this only happen if the new language is not equal to the old language?  (Also reiterating the comment about removing this variable and just clearing m_demangled)

http://reviews.llvm.org/D7302

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/