[Lldb-commits] [PATCH] D16746: Use an artifical namespace so that member vars do not hide local vars.
Siva Chandra via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 2 16:16:35 PST 2016
sivachandra marked an inline comment as done.
sivachandra added a comment.
Thanks, Greg and Sean for the review. The new version of the patch has tests also added. I also have an inline response to a comment by Sean.
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1381
@@ +1380,3 @@
+ clang::NamespaceDecl *namespace_decl = ast->GetUniqueNamespaceDeclaration(
+ name_unique_cstr, nullptr, m_ast_context);
+ if (namespace_decl)
> This bit where we're passing the AST context as an argument as well as the this pointer is a bit awkward. The common pattern when we have a class (like lldb::ClangASTContext) that provides useful functionality on top of another class (clang::ASTContext, in this case) is to add a method:
> static void GetUniqueNamespaceDeclaration(clang::ASTContext *, …)
> and then have the instance method call through to it transparently. Unless there's some reason why the frame's AST context is important, let's just do that here too.
I did not understand this comment fully. However, the frame's AST is important here because we want the new namespace decl to have the same clang::ExternalASTSource as that of the current frame. The ClangASTContext object |ast| above does not come with any AST set, and hence GetUniqueNamespaceDeclaration makes one up with external AST source set to ClangExternalASTSourceCallbacks instead of clang_internal::ClangASTSource. To avoid this, I added the new argument which specifies the AST to be used.
More information about the lldb-commits