Ok, that reasoning makes sense, I’ll see what i can do<br><div class="gmail_quote"><div dir="ltr">On Fri, Oct 26, 2018 at 9:25 AM Greg Clayton via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">clayborg added a comment.<br>
<br>
My issue with adding "exact_match" is it renders a few of the arguments useless:<br>
<br>
  size_t FindTypes(<br>
    const SymbolContext &sc, <br>
    llvm::StringRef name,<br>
    const CompilerDeclContext *parent_decl_ctx, <br>
    bool exact_match,<br>
    ...);<br>
<br>
With exact_match "parent_decl_ctx" is useless and possible the symbol context too. The FindTypes as written today was used for two cases which evolved over time:<br>
<br>
- finding types for users without needing to be fully qualified<br>
- finding types for the expression parser in a specific context<br>
<br>
To fulfill these two cases, arguments have been added over time. As we keep adding arguments we make the function even harder to implement for SymbolFile subclasses. So as long as we are cleaning things up, it might be time to factor this out by making changes now.<br>
<br>
Another complexity is that we can either search a single module or multiple modules when doing searches, and that is why we have Module::FindTypes_Impl(...) in since there is some work that needs to be done when we are searching by basename only (strip the context and search by basename, then filters matches afterward.<br>
<br>
That is why I was thinking we might want to make changes. Seems like having:<br>
<br>
  size_t FindTypesByBasename(<br>
    const SymbolContext &sc, <br>
    llvm::StringRef name,<br>
    const CompilerDeclContext *parent_decl_ctx<br>
    ...);<br>
<br>
  size_t FindTypesByFullname(llvm::StringRef name, ...);<br>
<br>
Clients of the first call must strip a typename to its identifier name prior to calling, and clients of the second can call with a fully qualified typename.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D53662" rel="noreferrer" target="_blank">https://reviews.llvm.org/D53662</a><br>
<br>
<br>
<br>
</blockquote></div>