paulherman added a comment.

In http://reviews.llvm.org/D12658#245466, @clayborg wrote:

> In http://reviews.llvm.org/D12658#244710, @paulherman wrote:
>
> > [WIP] Search variables based on clang::DeclContext and clang::Decl tree
> >
> > This revision fixes some of the comments. There are some things I'm not 
> > sure about. The problem is that at some point there will be the need to 
> > link decls with the object they represent (Function, Variable, CompileUnit, 
> > etc). Is the approach of getting the VariableSP from the TypeSystem the 
> > right one?
>
>
> It is fine because each TypeSystem does have a link to its SymbolFile, so yes 
> this will work.
>
> > Also, should ParseVariableDIE be moved to DWARFASTParser in order to create 
> > the decl there or should there only be a method 
> > CreateVariableDecl(VariableSP var)?
>
>
> No. ParseVariableDIE shouldn't be making the CompilerDecl at all. We should 
> do this only when we call the "Variable::GetDecl()". It should then route 
> this through the TypeSystem from the variable type and ask the type system 
> for the CompilerDecl. This will get routed to the SymbolFile and then to that 
> will get routed to the DWARFASTParser (DWARFASTParserClang for this case).


ParseVariableDIE does need at some point to get the CompilerDecl (but it gets 
it from Variable::GetDecl which calls DWARFASTParser) in order to link the Decl 
to the Variable. Is there a better solution to do this? Also, I think you're 
not looking at the latest diff as most of these have been addressed.


http://reviews.llvm.org/D12658



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

Reply via email to