<br><br><div class="gmail_quote">On Fri, Jun 10, 2011 at 9:18 AM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><br><div><div><div></div><div class="h5"><div>On Jun 9, 2011, at 6:18 PM, Kaelyn Uhrain wrote:</div><br><blockquote type="cite"><br><br><div class="gmail_quote">On Wed, Jun 1, 2011 at 9:56 AM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank">dgregor@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div><div><br><div><div>+ // Only perform the qualified lookups for C++</div><div>+ // FIXME: this breaks test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp</div><div>+ if (getLangOptions().CPlusPlus) {</div>
<div>+ TmpRes.suppressDiagnostics();</div><div>+ for (llvm::SmallPtrSet<IdentifierInfo*,</div><div>+ 16>::iterator QRI = QualifiedResults.begin(),</div><div>+ QRIEnd = QualifiedResults.end();</div>
<div>+ QRI != QRIEnd; ++QRI) {</div><div>+ for (llvm::SmallPtrSet<NamespaceDecl*,</div><div>+ 16>::iterator KNI = KnownNamespaces.begin(),</div><div>+ KNIEnd = KnownNamespaces.end();</div>
<div>+ KNI != KNIEnd; ++KNI) {</div><div>+ NameSpecifierAndSize NSS;</div><div>+ DeclContext *Ctx = dyn_cast<DeclContext>(*KNI);</div><div>+ TmpRes.clear();</div><div>+ TmpRes.setLookupName(*QRI);</div>
<div>+ if (!LookupQualifiedName(TmpRes, Ctx)) continue;</div><div>+</div><div>+ switch (TmpRes.getResultKind()) {</div><div>+ case LookupResult::Found:</div><div>+ case LookupResult::FoundOverloaded:</div>
<div>+ case LookupResult::FoundUnresolvedValue:</div><div>+ if (SpecifierMap.find(Ctx) != SpecifierMap.end()) {</div><div>+ NSS = SpecifierMap[Ctx];</div><div>+ } else {</div><div>
+ NSS = BuildSpecifier(Context, CurContextChain, Ctx);</div><div>+ SpecifierMap[Ctx] = NSS;</div><div>+ }</div><div>+ Consumer.addName((*QRI)->getName(), TmpRes.getAsSingle<NamedDecl>(),</div>
<div>+ ED + NSS.second, NSS.first);</div><div>+ break;</div><div>+ case LookupResult::NotFound:</div><div>+ case LookupResult::NotFoundInCurrentInstantiation:</div>
<div>+ case LookupResult::Ambiguous:</div><div>+ break;</div><div>+ }</div><div>+ }</div><div> }</div><div> }</div></div><div><br></div><div>I suggest always calling TmpRes.suppressDiagnostics() in the ambiguous case, otherwise you may end up getting ambiguity warnings. I don't know if that's the p4.cpp issue or not.</div>
<div><br></div><div>I suspect that we'll want to order the KnownNamespaces traversal from shortest NNS to longest NNS, so we can avoid performing name lookups (which are very expensive in the PCH case) when the NNS length + edit distance are worse than the best.</div>
</div></div></div></div></blockquote><div><br>Quick question about doing this: would it be more cost effective to use a std::multimap to allow iterating through the namespaces in ascending order of NNS length and exiting the loop once NNS length + edit distance are worse than the best, or to use an llvm::SmallVector instead and always looping over all of the namespaces, but skipping the lookup if (NNS length + ED) is too long? Or is there a better way than either option buried in the LLVM/Clang codebase (keeping in mind that there are three pieces of info to be kept together in some fashion: a DeclContext*, its NestedNameSpecifer*, and the "edit distance" of the NestedNameSpecifer*)?<br>
</div></div></blockquote><div><br></div></div></div>It seems like a SmallVector indexed by NNS length would be best.</div></div></blockquote><div><br>Indexing by NNS length might work (I keep thinking of the identifier edit distances that span a much larger range of values than the NNS length in most cases would)... just would have to a bit of funky nested SmallVector handling to deal with multiple NNSes having the same length.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div class="im"><div><br><blockquote type="cite">
<div class="gmail_quote"><div>Also, any thoughts on how to handle typo corrections in the presence of argument-dependent lookup (for solving the p4.cpp issue) would be appreciated...<br></div></div></blockquote></div><div>
<br></div></div><div>When ADL is involved, we shouldn't attempt any typo correction unless the call fails because there are no overload candidates. At that point (after overload resolution has failed), we could attempt typo correction and overload resolution again. I suggest tackling this separately.</div>
</div></blockquote><div><br>Yeah, I was just thinking on the way to work this morning that for this patch to just disable the namespace search when ADL is involved, then work on making typo correction more type-aware later.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><br><div><span style="white-space:pre-wrap"> </span>- Doug</div>
</div></blockquote></div><br>