<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 7, 2011, at 10:00 AM, Kaelyn Uhrain wrote:</div><blockquote type="cite"><font class="Apple-style-span" color="#000000"><br></font><div class="gmail_quote">On Mon, Jun 6, 2011 at 9:23 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
<div style="word-wrap:break-word"><br><div><div class="im"><div>On Jun 6, 2011, at 6:13 PM, Kaelyn Uhrain wrote:</div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap: break-word;"><div><div class="im"><font class="Apple-style-span" color="#007329"><br></font></div></div><div><div><div class="h5"><blockquote type="cite">
<div class="gmail_quote"><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><div><div><div><br></div><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></div></div></div></blockquote><div><br>Its not necessary since TmpRes.suppressDiagnostics() was called just before the beginning of the outer loop. And to make sure nothing sneaky is happening, I added an assertion that diagnostics are still suppressed to the ambiguous case, and at least for the test suite it was never triggered. I think the p4.cpp issue is related to the typo correction not (yet) being smart enough regarding argument dependent lookup.<br>
</div></div></blockquote><div><br></div></div></div><div>Each time the inner loop calls LookupQualifiedName, it's possible that we could end up with an ambiguity. That ambiguity needs to be suppressed, or it will be reported later. Perhaps there's more suppression later, but I'd feel a bit more comfortable if we always did it after the switch.</div>
</div></div></blockquote><div><br>My point was that suppressDiagnostics just sets a flag which, as far as I can tell is not modified by LookupQualifiedName or any of the other code within the inner loop. Unless I'm missing something (and some of the places where I've seen suppressDiagnostics() called miss the same thing), calling TmpRes.suppressDiagnostics() just before the loop starts will suppress diagnostics from anything in the loop--and without assigning the same value to an otherwise unchanged variable with every iteration.<br></div></div></blockquote><div><br></div>I see it it now. Thanks!</div><div><br><blockquote type="cite"><div class="gmail_quote"><div>
</div><blockquote class="gmail_quote" style="margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap: break-word;"><div><div class="im"><br><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex; position: static; z-index: auto; ">
<div style="word-wrap:break-word"><div><div><div><br></div><div>This is looking great, and we're getting close to the point where I'd like to put it into the tree! The major issues that I think need to be resolved before we can commit are:</div>
<div><span style="white-space:pre-wrap"> </span>- Getting the same behavior when using PCH and when not using PCH (the serialization issue mentioned at the top) <br></div></div></div></div></blockquote><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><div><div><span style="white-space:pre-wrap"> </span>- Testsuite failures</div></div></div></div></blockquote><div><br>As I said, the PCH matter requires a bit more learning and experimenting on my part since I've not done anything with them before. Fixing the test suite failures are a given for me, and have been a requirement for submitting the patch for inclusion into the tree. ;)<br>
</div></div></blockquote><div><br></div></div><div>I can take PCH off your todo list, since it'll be relatively quick for me to add that part.</div></div></div></blockquote><div><br>^.^ Any adjustments I should make to the data structures to facilitate PCH serialization and access?<br></div></div></blockquote></div><br><div>No, I'll deal with it.</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><div><br></div></body></html>