[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Sorry I am late to making comments on this patch.

This patch wants to add "llvm::StringRef scope" to the FindTypes calls, but this is what "const CompilerDeclContext &parent_decl_ctx" is actually for. If "parent_decl_ctx" is set, it is a declaration context that each type must exist in. So the CompilerDeclContext  might be something like "namespace foo" and it might have a parent decl context of "namespace bar". This is currently how filtering types that exist in a decl context works.

The "scope" seems redundant in light of this no? Also, a string based "scope" isn't accurate enough to correctly identify the exact scoping of a type as "a:🅱️:some_type" might be "namespace a -> class b -> some_type" or "namespace a -> namespace b -> some_type".

The one difficult thing about this is currently the "parent_decl_ctx" that is a parameter only works if the decl context comes from the current module. Each module creates types using its own TypeSystem subclass. Those types are vended as CompilerType objects which contain the TypeSystem pointer and a void * for the type. This allows each TypeSystem to create types. For clang compiled code we use lldb_private::TypeSystemClang and we create types as types in a clang::ASTContext. We can also specify declaration contexts by using CompilerDeclContext objects. Each CompilerDeclContext object has a "TypeSystem *" and a "void *" inside of it. Each expression makes its own TypeSystem as well, and one for each expression and one in the target. It should be possible to fix the issue of needing the compiler decl context to come from the current module by having the code that compares the decl context make by using:

  ConstString CompilerDeclContext::GetScopeQualifiedName() const;

Or we can modify the CompilerDeclContext class to get the current name:

  ConstString CompilerDeclContext::GetName() const; 

And then also add an API to return an enumeration for the kind of the declaration context (Namespace, Class, Struct, Union, etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627