<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 5, 2014 at 12:48 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Nov 4, 2014 at 4:08 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Oct 31, 2014 at 12:58 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br><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 dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Oct 31, 2014 at 11:19 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Oct 31, 2014 at 11:17 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr">Again reminds me that maybe an iterator idiom for typo corrections might be nice<br><br>(then peaking would be "std::next(i)->...")</div></blockquote></span><div><br>But that's up to you when/how you want to address that - given the current design, this looks plausible.<br><br>If there's a nice way to implement this function without having to change the object's internal state and then change it back, that'd be great (can the guts of getNextCorrection be factored in such a way as you could call "nextCorrectionIndex" as a non-mutating operation, then just use that to lookup the ValidatedCorrections array? Then the original getNextCorrection function could be something like CurrentTCIndex = nextCorrectionIndex(... ))<br></div></div></div></div></blockquote><div><br></div></span><div>This kind of thing (discovering a new piece of functionality that is needed, or that something has to be done differently) is exactly why I wanted to get most of the delayed typo correction usage fleshed out and working before refactoring the Consumer. ;) </div></div></div></div></blockquote><div><br></div></span><div>Sure enough (:</div><span><div> </div><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Though peeking would be necessity change the Consumer's state because generating the next correction candidate is done lazily, at least the first time through the correction stream... and since multiple passes through the correction stream are only ever necessary when there are multiple TypoExprs within the same expression (and even then, not always needed), I'd prefer keeping the laziness as much as possible.</div></div></div></div></blockquote></span><div><br>Fair - though I don't imagine it's /too/ important, if we're already on an error path. If I understand correctly, typo correction is already expensive enough that we make an effort not to perform it if we're not going to show it to a user (eg: if a warning fires that is currently disabled by the user, we make sure to check that the warning is disabled and skip doing the typo correction work) - so it might not be too important. In any case, I imagine it can be kept while still exposing an iterator-like API at some point.<br></div></div></div></div></blockquote><div><br></div></span><div>It's a little bit important because there are a couple of spots where I don't think it is strictly in an error path (IIRC Sema::ClassifyName attempts typo correction to determine whether there is an implicit int or an actual error, and a few other cases like that where whether there is an error is ambiguous and dependent on whether fixing a typo leads to a valid expression). </div></div></div></div></blockquote><div><br>Not sure I quite follow that. If the program is valid, typo correction can't make it not valid, and if the program is invalid typo correction can't make it valid (we still need to print an error and tell the user what we assumed they intended). But I guess I'm missing something.<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=""><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>To come back to my original comment - any lazy work being performed would still be performed on a peak, you'd just reset (or, ideally, not change in the first place) the actual "I'm up to this element" state. Not sure if that makes sense, depends how the laziness works (if it's always lazy/rebuilds everything, or if it's just lazy on the first pass, then simply references the existing stuff on the next pass).</div></div></div></div></blockquote><div><br></div></span><div>It's just lazy on the first pass, at which point the results are saved in a vector and reused on subsequent passes. That  peekNextCorrection resets the internal index is an artifact of it reusing the existing lazy mechanism to implement it, since the existing mechanism only generates a new TypoCorrection if the current index is at the end of the cache vector (peeking gets the next element then backs the index up so that getNextCorrection returns the peeked element before generating a new one). Quirks such as this are the kinds of things that should get cleaned up with a bit of refactoring.</div></div></div></div></blockquote><div><br>Fair enough - *scrolling back through the conversation* seems I/we originally started down this tangent with me asking whether this could be tidier. If you want to do it in followup cleanup patches, that's fine - consider this signed off on, then.<br><br>- David<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=""><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><span><font color="#888888"><br><br>- David<br> </font></span></div><span><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><span><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 class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Oct 29, 2014 at 12:49 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br></div></div><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>---<br>
 include/clang/Sema/SemaInternal.h | 15 +++++++++++++++<br>
 1 file changed, 15 insertions(+)<br>
<br>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>