jingham added a comment.

This change looks good to me too.

I don't think this will fix the parameter/local variable lookup inversion that 
we've been hacking around. The problem comes because to make an JITted object 
we can easily call we make a wrapper function with a simple argument list - 
that makes running it easy but means it doesn't share the original function's 
context.   Then we smuggle all the local Decls into the wrapper function using 
the FindExternalLexicalDecls from our ClangASTSource.  Unfortunately, that 
lookup gets consulted after the namespace/class lookup is done.  So names in 
the current class or namespace context shadow local variables/arguments.

This will clean up the AST Context's we make but sadly I don't think it will 
affect the expression parser name lookup issues.  If you wanted to test that 
proposition, you can do:

settings set target.experimental.inject-local-vars false

then try some tests where a local variable or parameter name shadows an ivar or 
type name in the current context.  That will find the wrong variable when this 
is false.  I'm sure there are also tests for this, but I don't remember where 
they are off-hand.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55571



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

Reply via email to