[Lldb-commits] [PATCH] D12044: Fix resolution conflict between global and class static variables in C++.

[Greg Clayton via lldb-commits]

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

See inlined comments.


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:7389
@@ -7388,3 +7388,3 @@
                         {
-                            VariableSP var_sp (ParseVariableDIE(sc, dwarf_cu, die, LLDB_INVALID_ADDRESS));
+                            VariableSP var_sp (ParseGlobalVariableDIE(sc, dwarf_cu, die, LLDB_INVALID_ADDRESS));
                             if (var_sp)
----------------
I would do this as:

```
const DWARFDebugInfoEntry *parent_die = GetDeclContextDIEContainingDIE(dwarf_cu, die);
if (parent_die->Tag() != DW_TAG_class_type && parent_die->Tag() != DW_TAG_structure_type)
{
    VariableSP var_sp (ParseVariableDIE(sc, dwarf_cu, die, LLDB_INVALID_ADDRESS));
    if (var_sp)
    {
        variables->AddVariableIfUnique (var_sp);
        ++vars_added;
    }
```

This will avoid parsing extra global variables by figuring out we don't need it _before_ we go and parse a global variable. 

I would rather not have the SymbolFileDWARF::ParseGlobalVariableDIE(...) function because it doesn't make sense at as global variables can exist in classes and structures and I would expect a function named SymbolFileDWARF::ParseGlobalVariableDIE(...) to parse that variable, but SymbolFileDWARF:: ParseVariableDIE(...) already does this so there is really no need for SymbolFileDWARF::ParseGlobalVariableDIE().

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:7413-7437
@@ -7412,2 +7412,27 @@
 
+VariableSP
+SymbolFileDWARF::ParseGlobalVariableDIE
+(
+    const SymbolContext& sc,
+    DWARFCompileUnit* dwarf_cu,
+    const DWARFDebugInfoEntry *die,
+    const lldb::addr_t func_low_pc
+)
+{
+    assert(sc.function == NULL);
+    assert(sc.comp_unit != NULL);
+
+    VariableSP var_sp(ParseVariableDIE(sc, dwarf_cu, die, func_low_pc)); 
+    
+    if (var_sp) 
+    {
+        const DWARFDebugInfoEntry *parent_die = GetDeclContextDIEContainingDIE(dwarf_cu, die);
+        if (parent_die->Tag() != DW_TAG_class_type && parent_die->Tag() != DW_TAG_structure_type)
+            return var_sp;
+        else
+            return VariableSP();
+    }
+
+    return var_sp;
+}
 
----------------
Remove this. See inlined comment above.

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:325-330
@@ -324,2 +324,8 @@
 
+    lldb::VariableSP        ParseGlobalVariableDIE(
+                                const lldb_private::SymbolContext& sc,
+                                DWARFCompileUnit* dwarf_cu,
+                                const DWARFDebugInfoEntry *die,
+                                const lldb::addr_t func_low_pc);
+
     size_t                  ParseVariables(
----------------
Remove this.


http://reviews.llvm.org/D12044