clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So one of two things needs to happen here:
- ClangASTContext::DeclContextCountDeclLevels() becomes a function that is on
ClangASTContext only and the opaque arguments get changed into
"clang::DeclContext *" args, then remove all DeclContextCountDeclLevels
functions from TypeSystem.h and GoASTContext.h.
- Change ClangASTContext::DeclContextCountDeclLevels() over to use
CompilerDeclContext objects for both "void *opaque_decl_ctx" and "void
*opaque_find_decl_ctx" and add any functions you need to do CompilerDeclContext
that is needed to complete the functionality of this function without
downcasting to clang objects.
So either make it generic, or clang specific.
I would vote for the first method since this functionality is very clang and
C++ specific. It doesn't seem like a nice general purpose function any other
language would use. If you do go the second route, you really need to abstract
the heck out of this and add any needed methods to CompilerDeclContext.
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1443
@@ +1442,3 @@
+ comp_sym_ctx =
frame->GetSymbolContext(lldb::eSymbolContextFunction|lldb::eSymbolContextBlock);
+ CompilerDeclContext compiler_decl_context = comp_sym_ctx.block
!= nullptr ? comp_sym_ctx.block->GetDeclContext() : CompilerDeclContext();
+
----------------
"compiler_decl_context" should probably be named "frame_decl_ctx" for clarity
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1467-1468
@@ +1466,4 @@
+ // Filter out class/instance methods.
+ if (decl_ctx.IsClassMethod(nullptr, nullptr,
nullptr))
+ continue;
+ sc_func_list.Append(sym_ctx);
----------------
Why are we pulling out class methods here?
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1485
@@ +1484,3 @@
+ };
+ auto initFuncDeclInfo = [this, compiler_decl_context,
ast](const SymbolContext &sym_ctx)
+ {
----------------
There is no need for this lamba, inline the code at the one and only place that
it is used.
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1505-1509
@@ +1504,7 @@
+ // scope and shadows the other.
+ fdi.m_func_decl_lvl =
+
ast->DeclContextCountDeclLevels(compiler_decl_context.GetOpaqueDeclContext(),
+
func_decl_context.GetOpaqueDeclContext(),
+
&fdi.m_function_name,
+
&fdi.m_copied_function_type);
+ }
----------------
Seems like you actually want this to be something like:
```
fdi.m_func_decl_lvl = func_decl_context.Depth(compiler_decl_context);
```
This would be a function on CompilerDeclContext that would calculate the depth
of a decl context if the first argument (compiler_decl_context) is contained
within the object (func_decl_context). This would be a useful API to add to
CompilerDeclContext. It can return an integer and return "-1" if the
compiler_decl_context ins't contained within func_decl_context, or zero or
greater if it is.
Then you would need to add more functions the CompilerDeclContext to get
"fdi.m_function_name" and "fdi.m_copied_function_type".
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1517
@@ +1516,3 @@
+ uint32_t num_indices = sc_func_list.GetSize();
+ std::vector<FuncDeclInfo> fdi_cache;
+ fdi_cache.reserve(num_indices);
----------------
wouldn't this be better as std::multimap<uint32_t, FuncDeclInfo> where the
uint32_t is the depth? Then you can change your foo loop at 1531 to just grab
the first entry in the map and iterate as long as the depth is the same...
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1524
@@ +1523,3 @@
+
+ struct FuncDeclInfo fdi = initFuncDeclInfo(sym_ctx);
+ fdi_cache.emplace_back(fdi);
----------------
inline the initFuncDeclInfo lambda here without needing a lambda
Repository:
rL LLVM
http://reviews.llvm.org/D15312
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
