I thought I had sent this to lldb-commits, but I’m not seeing it in the lldb-commits archives. So here is the request again…
“source list -n printf” wasn’t working on Linux because printf is a symbol and the code was only searching for functions.
The patch now searches for symbols if function searching fails and tries to find a symbol that lines up exactly with a function.
The Linux test suite passes with this patch. Please let me know if it’s ok to submit. Thanks.
The basic idea is okay.
It seems a little weird that if you've found no functions for a given name, you go looking for the symbols with that name by calling FindFunctions again, passing include_symbols=true. You're making another pass over all the functions even though you know that's not going to turn up anything. Why not just go look for symbols directly?
Good feedback. Would it make sense to add a FindFunctionSymbols() to
That seems reasonable. It should be simple, except there's a bunch of work we need to do to do fuzzy name matches (like folks expect if they look up foo::bar by name, it should match blah::foo::bar, etc. So encapsulating that for symbols makes sense as it does with functions. The FindFunctions with include_symbols=true is a separate thing, because it also does the work of coalescing symbols and functions so you don't end up with two copies of the same thing.
How does this look?
That looks okay.
The functions you added to CommandObjectSource (FindMatchingFunctions and FindMatchingFunctionSymbols) aren't really particular to CommandObjectSource, leading one to wonder if there were a better place to put them so they could be reused. IIUC their job is "take a possibly empty vector of strings which are names of modules, and run the two search functions on the subset of the full module list that matches the strings in the input vector". If we wanted to put them somewhere, there should probably be a module-filter-list that can be passed to the various ModuleList::Find* calls, which would either be a vector of string names or a ModuleSpecList. We don't do anything like that at present.
Cooking that up seems like a lot of trouble right now. We can probably hold off on that, but if we see this sort of construct getting used in other places, we should add such API's to ModuleList.