[PATCH v4 8/8] Wire up LookupMemberExpr to use the new TypoExpr.

[Kaelyn Takata]

On Fri, Oct 3, 2014 at 12:13 PM, David Blaikie <dblaikie at gmail.com> wrote:

> MemberTypoDiags looks like it'd be very well-written as a lambda (it's
> just forwarding state to the operator() call). This would imply changing
> the underlying API from unique_ptr<TypoDiagnosticGenerator>, removing
> TypoDiagnosticGenerator, and using std::function<void (const
> TypoCorrection &TC)> instead. The callers would then just pass in lambdas
> and std::function would provide the type erasure, dynamic destruction, etc.
>
> MemberExprTypoRecovery is a bit longer, but still does seem like a lambda might suite (& even if it doesn't - writing it as a functor (which it already is - but you would/could strip off the base class and the virtuality of the op()) and having the API use std::function is a bit more flexible)
>
> Making MemberExprTypoRecovery a simple functor wouldn't work because it
needs to capture state that isn't passed in when called (the four arguments
to its constructor). As for TypoDiagnosticGenerator, MemberTypoDiags has
the same problem of needing to capture state, and while it is short enough
that writing it as a lambda that captures the needed state wouldn't be too
painful, some of the subsequent TypoDiagnosticGenerator descendants both
capture state and are longer / more complex than MemberTypoDiags
(e.g. EmptyLookupTypoDiags in
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140818/113266.html
is
longer than MemberExprTypoRecovery and has state that goes beyond simply
capturing variables).

> Otherwise this all looks /fairly/ straightforward (again, I don't have all the domain knowledge to judge this - the getDeclFromExpr and its use might use comments (and/or the added code that uses it might be pulled out into another function with a self-documenting name)). But again, some of this is probably just my unfamiliarity with the code.
>
> getDeclFromExpr does exactly what it's name says. :) I've added the
following comment to the one call to getDeclFromExpr to explain what is
being done and why (and added a bit of whitespace around the code block,
which had been moved to the EmitAllDiagnostics helper added in response to
your comments on patch #5):

        // Extract the NamedDecl from the transformed TypoExpr and add it
to the
        // TypoCorrection, replacing the existing decls. This ensures the
right
        // NamedDecl is used in diagnostics e.g. in the case where overload
        // resolution was used to select one from several possible decls
that
        // had been stored in the TypoCorrection.
        if (auto *ND = getDeclFromExpr(
                Replacement.isInvalid() ? nullptr : Replacement.get()))
          TC.setCorrectionDecl(ND);


> On Thu, Sep 25, 2014 at 2:00 PM, Kaelyn Takata <rikka at google.com> wrote:
>
>> ping
>>
>> On Tue, Aug 26, 2014 at 11:06 AM, Kaelyn Takata <rikka at google.com> wrote:
>>
>>> +dblaikie
>>>
>>>
>>> On Thu, Jul 31, 2014 at 1:20 PM, Kaelyn Takata <rikka at google.com> wrote:
>>>
>>>>
>>>> This includes adding the new TypoExpr-based lazy typo correction to
>>>> LookupMemberExprInRecord as an alternative to the existing eager typo
>>>> correction.
>>>> ---
>>>>  lib/Sema/SemaExprCXX.cpp                 |  40 ++++++++++-
>>>>  lib/Sema/SemaExprMember.cpp              | 116
>>>> ++++++++++++++++++++++++++++---
>>>>  test/SemaCXX/arrow-operator.cpp          |   5 +-
>>>>  test/SemaCXX/typo-correction-delayed.cpp |  32 +++++++++
>>>>  test/SemaCXX/typo-correction-pt2.cpp     |   2 +-
>>>>  test/SemaCXX/typo-correction.cpp         |  10 +--
>>>>  6 files changed, 186 insertions(+), 19 deletions(-)
>>>>  create mode 100644 test/SemaCXX/typo-correction-delayed.cpp
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141007/7b0bcacb/attachment.html>