clayborg added inline comments.
================
Comment at: lldb/include/lldb/Core/Module.h:435
+ /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a".
+ lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match);
----------------
aprantl wrote:
> clayborg wrote:
> > aprantl wrote:
> > > Why is this not taking a TypeQuery that wraps a name?
> > This function doesn't need to exist as long as the "TypeSP
> > TypeQuery::FindFirstType(Module)" function is ok to have around.
> It would be good to have some consistency in the interface:
> - Either TypeQuery provides Find* methods that take a Module
> - or Module provides Find* methods that take a TypeQuery.
>
> Why does this interface exist instead of having a "FindFirstOnly" flag in
> TypeQuery? Is it because of the overhead of having to dig the type out of a
> TypeMap?
>
> Otherwise we could just have one method
>
> Module::FindTypes(const TypeQuery &query, TypeResults &results);
>
> that can be called like this
> ```
> module_sp->FindTypes(TypeQuery::createFindFirstByName("name", e_exact_match),
> result);
> TypeSP type = result.GetTypeAtIndex(0); /// better have a special method for
> this too
> ```
See what you think of the current code and let me know any changes you would
like to see.
================
Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93
+ void GetCompilerContext(
+ llvm::SmallVectorImpl<lldb_private::CompilerContext> &context) const;
+
----------------
clayborg wrote:
> aprantl wrote:
> > aprantl wrote:
> > > Why can't this be a return value? The context objects are tiny.
> > ping?
> Will change!
I looked around LLVM code and there are many accessors that use this technique.
Otherwise we need to enforce an exact SmallVector type as the return code and
we can't use "llvm::SmallVectorImpl<lldb_private::CompilerContext>". I can
change this to return a "llvm::SmallVector<CompilerContext, 4>" if needed? But
that locks us into a specific small vector with a specific size of preached
items.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137900/new/
https://reviews.llvm.org/D137900
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
