[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

[Jim Ingham via Phabricator via lldb-commits]

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