Rework breakpoint language filtering to use the symbol context's language.

[dawn]

Summary

This patch reworks the breakpoint filter-by-language patch to use the symbol context instead of trying to guess the language solely from the symbol's name. This has the advantage that symbols compiled with debug info will have their actual language known. Symbols without debug info will still do the same "guess"ing because Symbol::GetLanguage() is implemented using Mangled::GuessLanguage(). The recognition of ObjC names was merged into Mangled::GuessLanguage.

[jingham]

The part of this fix which is using info in the SymbolContext to make the language detection more accurate is fine.

We have to do something better about how we detect the language from a mangled name. The target knows some that it should be able to contribute, but there's no clear way to do that now. But you aren't making that any worse (except the one inline comment about cstring_is_mangled above, which will need to get fixed) so I don't think you are obligated to solve that problem now.

[dawn]

In D15326#305163, @jingham wrote:

The part of this fix which is using info in the SymbolContext to make the language detection more accurate is fine.

We have to do something better about how we detect the language from a mangled name. The target knows some that it should be able to contribute, but there's no clear way to do that now. But you aren't making that any worse (except the one inline comment about cstring_is_mangled above, which will need to get fixed) so I don't think you are obligated to solve that problem now.

Can we please accept this as is? It helps *a lot* - we know the *exact* language whenever we have debug info for an identifier. I've been able to remove my #if 0's around the filtering code with this patch. As you say, it doesn't make anything worse, and for many of us makes, things are far better. Why would you reject it?

The name mangling issue is a whole other beast - it was a flawed assumption from the start (and I complained about it then). I'm simply trying to make the best with what we have.

[dawn]

(Added Greg - he wrote Mangled::GetLanguage - now GuessLanguage).

This patch removes the dependence on determining language from the name mangling for 99% of cases (there's still the much less common problem for symbols since Symbol 's GetLanguage calls Mangled::GuessLanguage). It is a win win.

[dawn]

Please reconsider. Thanks.

[clayborg]

Just fix the comment as mentioned in the inline comments and this is ready. Jim should OK this as well.

include/lldb/Target/Language.h

176

Fix comment to say "so if LanguageIsC(language)" since the variable below is "language".

[dawn]

Thanks Greg. Will fix comment in commit.

@Jim, can you accept please?

[jingham]

"cstring_is_mangled" does NOT mean that the string is a C++ mangled string, it means it is ANY kind of mangled string - you can tell because it is used to determine whether to populate the m_mangled and m_demangled components of the Mangled structure, which it should do regardless of the language the compiler happens to be mangling . If you look at the Github sources with Swift support, you will see that this returns true for both C++ mangled names and Swift mangled names, as it should.

It looks like you are using a function that had this error originally, so it is not your fault per se. We were probably getting away with it because it was never used in a case where its result mattered (it isn't used much...) But you are now using it somewhere where the incorrect usage would matter. So it needs to be fixed before the patch is correct.

For the nonce, it is probably good enough for Mangled::GuessLanguage to return C++ if the mangling scheme is Itanium or MSVC. We definitely have to figure out a better way to this, so a comment to that effect would also be in order.

[dawn]

ping?

[jingham]

This patch is not acceptable as is. It enshrines an incorrect usage of the function cstring_is_mangled which we shouldn’t do. As I said in the previous comment, it’s okay to come up with a short term fix, and check the mangled flavor against Itanium and MSVC. So it shouldn’t be hard to replace the errant usage. Do that and its fine to go in.

Jim

[dawn]

Updated patch which changes Mangled::GuessLanguage to use the same algorithm for detecting the language from the mangled name as was added to LanguageRuntime::GuessLanguageForSymbolByName.

Jim, sorry for the previous ping - I missed your comment which included your objections. Hopefully the patch is acceptable now?

[jingham]

That's great.

include/lldb/Symbol/SymbolContext.h

     uint32_t
     GetResolvedMask () const;
 
+    lldb::LanguageType
+    GetLanguage () const;
+
     //------------------------------------------------------------------
     /// Find a block that defines the function represented by this
     /// symbol context.

include/lldb/Target/Language.h

     static bool
     LanguageIsPascal (lldb::LanguageType language);
     
+    // return the primary language, so if LanguageIsC(l), return eLanguageTypeC, etc.
+    static lldb::LanguageType
+    GetPrimaryLanguage (lldb::LanguageType language);
+
     static void
     GetLanguagesSupportingTypeSystems (std::set<lldb::LanguageType> &languages,
                                        std::set<lldb::LanguageType> &languages_for_expressions);

include/lldb/Target/LanguageRuntime.h

         return m_process;
     }
     
-    static lldb::LanguageType
-    GuessLanguageForSymbolByName (Target &target, const char *symbol_name);
-    
     Target&
     GetTargetRef()
     {

source/Breakpoint/BreakpointResolverName.cpp

 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
-#include "lldb/Target/LanguageRuntime.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 using namespace lldb;

     // If the filter specifies a Compilation Unit, remove the ones that don't pass at this point.
     if (filter_by_cu || filter_by_language)
     {
-        Target &target = m_breakpoint->GetTarget();
-        
         uint32_t num_functions = func_list.GetSize();
         
         for (size_t idx = 0; idx < num_functions; idx++)

             
             if (filter_by_language)
             {
-                const char *name = sc.GetFunctionName(Mangled::ePreferMangled).AsCString();
-                if (name)
+                LanguageType sym_language = sc.GetLanguage();
+                if ((Language::GetPrimaryLanguage(sym_language) !=
+                     Language::GetPrimaryLanguage(m_language)) &&
+                    (sym_language != eLanguageTypeUnknown))
                 {
-                    LanguageType sym_language = LanguageRuntime::GuessLanguageForSymbolByName(target, name);
-                    if (Language::LanguageIsC(m_language) ||
-                        Language::LanguageIsPascal(m_language))
-                    {
-                        // We don't currently have a way to say "This symbol name is C" so for now, C means
-                        // not ObjC and not C++, etc...
-                        if (sym_language == eLanguageTypeC_plus_plus
-                            || sym_language == eLanguageTypeObjC
-                            || sym_language == eLanguageTypeSwift)
-                        {
-                            remove_it = true;
-                        }
-                    }
-                    else if (sym_language != m_language)
-                    {
-                        // Note: This code prevents us from being able to find symbols
-                        // like 'printf' if the target language's option is set.  It
-                        // would be better to limit this filtering to only when the
-                        // breakpoint's language option is set (and not the target's),
-                        // but we can't know if m_language was set from the target or
-                        // breakpoint option.
-                        remove_it = true;
-                    }
+                    remove_it = true;
                 }
             }
             

source/Core/Mangled.cpp

 #include "lldb/Core/Stream.h"
 #include "lldb/Core/Timer.h"
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include <ctype.h>
 #include <string.h>
 #include <stdlib.h>

     return m_mangled.MemorySize() + m_demangled.MemorySize();
 }
 
+//----------------------------------------------------------------------
+// We "guess" the language because we can't determine a symbol's language
+// from it's name.  For example, a Pascal symbol can be mangled using the
+// C++ Itanium scheme, and defined in a compilation unit within the same
+// module as other C++ units.  In addition, different targets could have
+// different ways of mangling names from a given language, likewise the
+// compilation units within those targets.
+//----------------------------------------------------------------------
 lldb::LanguageType
 Mangled::GuessLanguage () const
 {

     {
         if (GetDemangledName(lldb::eLanguageTypeUnknown))
         {
-            if (cstring_is_mangled(mangled.GetCString()))
+            const char *mangled_name = mangled.GetCString();
+            if (CPlusPlusLanguage::IsCPPMangledName(mangled_name))
                 return lldb::eLanguageTypeC_plus_plus;
+            else if (ObjCLanguage::IsPossibleObjCMethodName(mangled_name))
+                return lldb::eLanguageTypeObjC;
         }
     }
-    return  lldb::eLanguageTypeUnknown;
+    return lldb::eLanguageTypeUnknown;
 }
 
 //----------------------------------------------------------------------

source/Symbol/SymbolContext.cpp

     return false;
 }
 
+LanguageType
+SymbolContext::GetLanguage () const
+{
+    LanguageType lang;
+    if (function &&
+        (lang = function->GetLanguage()) != eLanguageTypeUnknown)
+    {
+        return lang;
+    }
+    else if (variable &&
+             (lang = variable->GetLanguage()) != eLanguageTypeUnknown)
+    {
+        return lang;
+    }
+    else if (symbol &&
+             (lang = symbol->GetLanguage()) != eLanguageTypeUnknown)
+    {
+        return lang;
+    }
+    else if (comp_unit &&
+             (lang = comp_unit->GetLanguage()) != eLanguageTypeUnknown)
+    {
+        return lang;
+    }
+    else if (symbol)
+    {
+        // If all else fails, try to guess the language from the name.
+        return symbol->GetMangled().GuessLanguage();
+    }
+    return eLanguageTypeUnknown;
+}
+
 bool
 SymbolContext::GetParentOfInlinedScope (const Address &curr_frame_pc, 
                                         SymbolContext &next_frame_sc, 

source/Target/Language.cpp

     }
 }
 
+LanguageType
+Language::GetPrimaryLanguage (LanguageType language)
+{
+    switch (language)
+    {
+        case eLanguageTypeC_plus_plus:
+        case eLanguageTypeC_plus_plus_03:
+        case eLanguageTypeC_plus_plus_11:
+        case eLanguageTypeC_plus_plus_14:
+            return eLanguageTypeC_plus_plus;
+        case eLanguageTypeC:
+        case eLanguageTypeC89:
+        case eLanguageTypeC99:
+        case eLanguageTypeC11:
+            return eLanguageTypeC;
+        case eLanguageTypeObjC:
+        case eLanguageTypeObjC_plus_plus:
+            return eLanguageTypeObjC;
+        case eLanguageTypePascal83:
+        case eLanguageTypeCobol74:
+        case eLanguageTypeCobol85:
+        case eLanguageTypeFortran77:
+        case eLanguageTypeFortran90:
+        case eLanguageTypeFortran95:
+        case eLanguageTypeFortran03:
+        case eLanguageTypeFortran08:
+        case eLanguageTypeAda83:
+        case eLanguageTypeAda95:
+        case eLanguageTypeModula2:
+        case eLanguageTypeJava:
+        case eLanguageTypePLI:
+        case eLanguageTypeUPC:
+        case eLanguageTypeD:
+        case eLanguageTypePython:
+        case eLanguageTypeOpenCL:
+        case eLanguageTypeGo:
+        case eLanguageTypeModula3:
+        case eLanguageTypeHaskell:
+        case eLanguageTypeOCaml:
+        case eLanguageTypeRust:
+        case eLanguageTypeSwift:
+        case eLanguageTypeJulia:
+        case eLanguageTypeDylan:
+        case eLanguageTypeMipsAssembler:
+        case eLanguageTypeExtRenderScript:
+        case eLanguageTypeUnknown:
+        default:
+            return language;
+    }
+}
+
 void
 Language::GetLanguagesSupportingTypeSystems (std::set<lldb::LanguageType> &languages,
                                              std::set<lldb::LanguageType> &languages_for_expressions)

source/Target/LanguageRuntime.cpp

 {
     return m_process->GetTarget().GetSearchFilterForModule(NULL);
 }
-
-lldb::LanguageType
-LanguageRuntime::GuessLanguageForSymbolByName (Target &target, const char *symbol_name)
-{
-    // We "guess" the language because we can't determine a symbol's language from it's name.
-    // For example, a Pascal symbol can be mangled using the C++ Itanium scheme, and defined
-    // in a compilation unit within the same module as other C++ units.
-    //
-    // In addition, different targets could have different ways of mangling names from a given
-    // language, likewise compilation units within those targets.  It would help to be able to
-    // ask the various LanguageRuntime plugin instances for this target to recognize the name,
-    // but right now the plugin instances depend on the process, not the target.  That is
-    // unfortunate, because to use this for filtering breakpoints by language, we need to know
-    // the "language for symbol-name" prior to running.  So we'd have to make a
-    // "LanguageRuntimeTarget" and "LanguageRuntimeProcess", and direct the questions that don't
-    // need a running process to the former, and that do to the latter.
-    //
-    // That's more work than I want to do for this feature.
-    if (CPlusPlusLanguage::IsCPPMangledName (symbol_name))
-        return eLanguageTypeC_plus_plus;
-    else if (ObjCLanguage::IsPossibleObjCMethodName (symbol_name))
-        return eLanguageTypeObjC;
-    else
-        return eLanguageTypeUnknown;
-}