shafik added inline comments.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3281-3282
     if ((parent_tag == DW_TAG_compile_unit ||
-         parent_tag == DW_TAG_partial_unit) &&
+         parent_tag == DW_TAG_partial_unit ||
+         parent_tag == DW_TAG_namespace) &&
         Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU())))
----------------
clayborg wrote:
> tonkosi wrote:
> > clayborg wrote:
> > > I think we should just always call this function in regardless of what 
> > > the parent variable tag is since what happens if the parent tag is 
> > > another value decl context like DW_TAG_class_type, DW_TAG_structure_type? 
> > > 
> > > The call to GetDWARFDeclContext(die) below will calculate the 
> > > DWARFDeclContext for a DIE and then construct an appropriate qualified 
> > > name, so we can just always do this so I would suggest just making this:
> > > 
> > > ```
> > > if (Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU())))
> > >   mangled = 
> > > GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();
> > > ```
> > > Then we should always get this right all the time. It doesn't actually 
> > > make sense to call this if the parent is the DW_TAG_compile_unit or 
> > > DW_TAG_partial_unit because then there is no decl context to add to the 
> > > variable name.
> > I tried to call `GetDWARFDeclContext(die)` in a general case, but it seems 
> > that `GetQualifiedName` will append `::` 
> > ([source](https://github.com/llvm/llvm-project/blob/5015f250894d3d97917d858850fae7960923a4ae/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp#L22))
> >  in front of the variable name if there's no other declaration context.
> > 
> > As a result, LLDB will display local variables and function arguments as 
> > `::var`. Any suggestion to deal with that?
> > 
> > I also noticed that with the current change it would be possible to get 
> > variables in anonymous namespaces, e.g. for the following source
> > 
> > ```
> > namespace ns {
> > namespace {
> > const int var = 1;
> > }
> > }
> > ```
> > 
> > call to `SBTarget::FindGlobalVariables("ns::(anonymous namespace)::var")` 
> > will succeed. Is that OK?
> GetQualifiedNameAsConstString could take an extra argument that specifies if 
> we should prepend the "::" to the root namespace.
> 
> I am fine with a lookup of "ns::(anonymous namespace)::var" succeeding.
You should document the anonymous namespace case in a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112147/new/

https://reviews.llvm.org/D112147

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to