aprantl added a comment. I have only two concerns left:
1. IMHO it would be better if `find_one` (which I think of as a search *input*) would live in TypeQuery instead of TypeResult. It feels wrong to use TypeResult to pass something *in*. 2. It's inconsistent that Module has a FindTypes(TypeQuery) method and TypeQuery has a Find(First)Type(Module) method. Could we just move `find_one` into `TypeQuery` and get rid of the `Find` methods in `TypeQuery`. Let me know if I'm misunderstanding something. ================ Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93 + void GetCompilerContext( + llvm::SmallVectorImpl<lldb_private::CompilerContext> &context) const; + ---------------- clayborg wrote: > clayborg wrote: > > 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. > change "preached" to "preset" above... Thanks for checking — that makes sense. ================ Comment at: lldb/source/Symbol/Type.cpp:149 + module->FindTypes(*this, results); + return results.GetTypeMap().FirstType(); +} ---------------- IMHO it would be better if `find_one` (which I think of as a search *input*) would live in TypeQuery instead of TypeResult. It feels wrong to use TypeResult to pass something *in*. 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
