[PATCH v3 9/9] Wire up LookupMemberExpr to use the new TypoExpr.

[Kaelyn Takata]

On Fri, Jul 25, 2014 at 3:03 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> +  if (TE && SemaRef.getLangOpts().CPlusPlus) {
> +    // TODO: C cannot handle TypoExpr nodes because the C code paths do
> not know
> +    // what to do with dependent types e.g. on the LHS of an assigment.
> +    *TE = SemaRef.CorrectTypoDelayed(
>
> Does this recover correctly if CorrectTypoDelayed returns nullptr? I'd
> have expected that you should carry on and produce an error in this case.
>

Yes it does in that the behavior of LookupMemberExprInRecord when
CorrectTypoDelayed returns nullptr is identical to the behavior of when
CorrectTypo fails: LookupMemberExprInRecord returns false and
LookupMemberExpr returns an ExprResult containing a nullptr. In both cases,
the final recovery is based on the LookupResult having been cleared.

I've dropped the superfluous ?: in LookupMemberExpr and expanded the
existing comment to include a description of the error case since the
comment had just said that returning valid-but-null meant the LookupResult
was filled in.


>
> +        SemaRef.AddMethodCandidate(
> +            DeclAccessPair::make(ND, AS_none),
> BaseType.getNonReferenceType(),
> +
>  /*MemberRef.get()->Classify(SemaRef.Context)*/Expr::Classification::makeSimpleLValue(),
> Args, CandidateSet);
>
> Commented-out code?
>

Whoops! Thanks.


> +      // Perform overload resolution.
> +      OverloadCandidateSet::iterator Best;
> +      auto result = CandidateSet.BestViableFunction(
> +          SemaRef, CE->getLocStart(), Best);
>
> Do you need to do this here? Can you instead simply build an unresolved
> overload set and allow the TreeTransform to do overload resolution when it
> transforms the parent, or is there some subtlety that makes this necessary?
>

Yes the resolution has to be done here. The "subtlety" is that letting the
call to BuildMemberReferenceExpr return an UnresolvedMemberExpr when the
parent expr is a CallExpr leads to incorrect diagnostic notes, e.g. from
typo-correction-pt2.cpp:

namespace PR13387 {
struct A {
  void CreateFoo(float, float);
  void CreateBar(float, float);
};
struct B : A {
  using A::CreateFoo;
  void CreateFoo(int, int);  // expected-note {{'CreateFoo' declared here}}
};
void f(B &x) {
  x.Createfoo(0,0);  // expected-error {{no member named 'Createfoo' in
'PR13387::B'; did you mean 'CreateFoo'?}}
}
}

Without the overload resolution, the note for CreateFoo points to the wrong
CreateFoo (it ends up pointing to the first member with that name):

test/SemaCXX/typo-correction-pt2.cpp:26:8: note: 'CreateFoo' declared here
  void CreateFoo(float, float);

With the overload resolution, the note points to the correct Createfoo:

test/SemaCXX/typo-correction-pt2.cpp:31:8: note: 'CreateFoo' declared here
  void CreateFoo(int, int);  // expected-note {{'CreateFoo' declared here}}

I've added a comment explaining why the overload resolution is being
performed here.


>
> +  something(obj.toobat,   // expected-error {{did you mean 'foobar'?}}
> +            obj.toobat);  // expected-error {{did you mean 'toobad'?}}
>
> I love this testcase =)
>

^_^


> On Mon, Jul 14, 2014 at 4:55 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/SemaExprMember.cpp              | 149
>> +++++++++++++++++++++++++++++--
>>  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 +--
>>  5 files changed, 182 insertions(+), 16 deletions(-)
>>  create mode 100644 test/SemaCXX/typo-correction-delayed.cpp
>>
>>
>> _______________________________________________
>> 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/20140730/73c0413e/attachment.html>