<div dir="ltr">On Mon, May 6, 2013 at 4:19 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=nlewycky@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">nlewycky@google.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On 9 April 2013 15:59, Kaelyn Uhrain <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=rikka@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">rikka@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">The patch looks good to me for the most part. One thing you may want to try though: the TypoCorrection *should* already contain all of the NamedDecls needed for overload resolution (TypeCorrection::getCorrectionDecl just returns the first NamedDecl if there are any associated with the TypoCorrection, i.e. the correction is for an existing declaration and is not a keyword), so you ought to be able to avoid the additional name lookup by doing:<div>
<br></div><div>for (TypoCorrection::decl_iterator DI = Corrected.begin(), DIEnd = Corrected.end(); DI != DIEnd; ++DI) {</div><div> R.addDecl(*DI);</div><div>}</div><div><br></div><div>instead of calling "SemaRef.LookupQualifiedName(R, DC);"... that the function was only adding the first NamedDecl to the LookupResult instead of all of them was because I missed it when I improved the handling of typo corrections in the presence of overloaded names.</div>
</div></blockquote><div><br></div></div><div>Yes, the TypoCorrection does contain them all. I didn't do that because I thought I would need to do a lookup on the correct name in order to trigger instantiation of dependent return types, but that wasn't correct, and I've included a testcase for that. What really happens is that we build an UnresolvedMemberExpr now and then the necessary template instantiations are performed in BuildCallToMemberFunction, after we have all the arguments. (As with all things, this makes perfect sense in retrospect.)</div>
</div></div></div></blockquote><div><br></div><div style>Ah yes, that does indeed make sense. I had a feeling it was related to when typo correction happened for members being used. :)</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div>And one important difference I thought of while writing the above: although it probably doesn't affect LookupMemberExprInRecord right now since member lookups occur before the function call information is available, the NamedDecls in the TypoCorrection object would not necessarily be the same set returned by LookupQualifiedName... they would have been filtered by the CorrectionCandidateCallback object passed to CorrectTypo, reducing the work that the overload resolution would have to do later and possibly avoiding it altogether if the callback has enough information to exclude all but one candidate. Calling LookupQualifiedName could lead to overload resolution being done because it found multiple NamedDecls even when CorrectTypo returned a correction with a single NamedDecl. Also, the TypoCorrection may include a NestedNameSpecifier that is needed for the the name to be visible from the given DeclContext (the "DC" variable), and in those situations the call to LookupQualifiedName on the correction's DeclarationName would fail.<br>
</div></div></blockquote><div><br></div></div><div>Yep, thanks for pointing that out. I took a look and while I'm not certain, I also think it doesn't affect LookupMemberExprInRecord.<span style="color:black"> </span></div>
</div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>
New patch attached!</div></div></div></div></blockquote><div><br></div><div style>You're welcome! The new patch looks spot on to me. :) </div><div style><br></div><div style>Cheers,</div><div style>Kaelyn</div><div style>
<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="HOEnZb"><font color="#888888"><div>
<br></div><div>Nick</div></font></span><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div>
<div><div class="gmail_extra"><span style="color:rgb(80,0,80)">On Mon, Apr 8, 2013 at 4:35 PM, Nick Lewycky </span><span dir="ltr" style="color:rgb(80,0,80)"><<a href="mailto:nlewycky@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=nlewycky@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">nlewycky@google.com</a>></span><span style="color:rgb(80,0,80)"> wrote:</span><br>
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div dir="ltr">
The problem is best explained with a testcase:<div>
<br></div><div><div>struct A {};</div><div>struct B {};</div><div><br></div><div>struct S {</div><div> void method(A*);</div><div> void method(B*);</div><div>
};</div><div><br></div><div>void test() {</div><div> B b;</div><div> S s;</div><div> s.methodd(&b);</div><div>}</div></div><div><br></div><div>we currently typo-correct the "s.methodd" to "s.method" but refer to the NamedDecl* for "void method(A*);" both in the note and when continuing to parse. That in turn causes an extra diagnostics (can't convert B* to A*), but worse in my mind is the fact that the parser isn't recovering as though the suggested replacement text were applied.</div>
<div><br></div><div>Patch attached, now we recover by returning a LookupResult with all the overload candidates found by looking up the typo-corrected name, and we don't emit the note pointing to the previous decl(s) in case of overloaded results. Please review closely, I'm not very familiar with this part of clang.</div>
<span><font color="#888888">
<div><br></div><div>Nick</div><div><br></div></font></span></div>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=cfe-commits@cs.uiuc.edu&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div></div>