Page MenuHomePhabricator

Fix resolution conflict between global and class static variables in C++.
ClosedPublic

Authored by paulherman on Aug 14 2015, 4:54 PM.

Details

Summary

Consider having a global variable 'a' and a static variable with the same name inside a class. This resolves the arbitrary choice when resolving the name 'a'.

Diff Detail

Event Timeline

paulherman updated this revision to Diff 32202.Aug 14 2015, 4:54 PM
paulherman retitled this revision from to Fix resolution conflict between global and class static variables in C/C++..
paulherman updated this object.
paulherman added a subscriber: lldb-commits.
chaoren retitled this revision from Fix resolution conflict between global and class static variables in C/C++. to Fix resolution conflict between global and class static variables in C++..Aug 14 2015, 4:58 PM
clayborg requested changes to this revision.Aug 17 2015, 9:33 AM
clayborg edited edge metadata.

See inlined comments.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
7389

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().

7413–7437

Remove this. See inlined comment above.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
325–330 ↗(On Diff #32202)

Remove this.

This revision now requires changes to proceed.Aug 17 2015, 9:33 AM

Please make sure that the following works after your changes:

(lldb) r
Process 35421 launched: '/private/tmp/a.out' (x86_64)
Process 35421 stopped
* thread #1: tid = 0xb659be, 0x0000000100000f9d a.out main + 13, stop reason = breakpoint 1.1, queue = com.apple.main-thread
    frame #0: 0x0000000100000f9d a.out main + 13 at main.cpp:24
   21  	
   22  	int main()
   23  	{
-> 24  	    return 0; // break here
   25  	}
(lldb) target variable 
Global variables for /tmp/main.cpp in /private/tmp/a.out:
(int) A::a = 1111
(int) B::a = 2222
(int) C::a = 3333
(int) ::a = 4444
paulherman updated this revision to Diff 32345.EditedAug 17 2015, 3:05 PM
paulherman edited edge metadata.

I've changed the commit so that ParseVariableDIE now detects if a variable is static inside a class or struct. Now "target variable" works since the variables are not removed from the variable list of the compile unit, but filtered when searching for a specific variable.

clayborg accepted this revision.Aug 17 2015, 4:45 PM
clayborg edited edge metadata.

Looks fine.

This revision is now accepted and ready to land.Aug 17 2015, 4:45 PM
paulherman closed this revision.Aug 20 2015, 12:29 PM