[PATCH v2 3/6] Add a few small helper methods to the TypoCorrectionConsumer.

David Blaikie dblaikie at gmail.com
Tue Nov 4 16:08:05 PST 2014

On Fri, Oct 31, 2014 at 12:58 PM, Kaelyn Takata <rikka at google.com> wrote:

> On Fri, Oct 31, 2014 at 11:19 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
>> On Fri, Oct 31, 2014 at 11:17 AM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>> Again reminds me that maybe an iterator idiom for typo corrections might
>>> be nice
>>> (then peaking would be "std::next(i)->...")
>> But that's up to you when/how you want to address that - given the
>> current design, this looks plausible.
>> 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(... ))
> 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. ;)

Sure enough (:

> 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.

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

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).

- David

>>> On Wed, Oct 29, 2014 at 12:49 PM, Kaelyn Takata <rikka at google.com>
>>> wrote:
>>>> ---
>>>>  include/clang/Sema/SemaInternal.h | 15 +++++++++++++++
>>>>  1 file changed, 15 insertions(+)
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141104/069b9d08/attachment.html>

More information about the cfe-commits mailing list