[PATCH] Instantiate UnresolvedLookupExpr to MemberExpr when appropriate

[Richard Smith]

On Tue, Dec 16, 2014 at 4:07 PM, Reid Kleckner <rnk at google.com> wrote:

> Hi rsmith,
>
> During the initial template parse for this code, 'member' is unresolved
> and we don't know anything about it:
>
>   struct A { int member };
>   template <typename T>
>   struct B : public T {
>     using T::member;
>     static void f() {
>       (void)member; // Could be static or non-static.
>     }
>   };
>   template class B<A>;
>
> The pattern declaration contains an UnresolvedLookupExpr rather than an
> UnresolvedMemberExpr.


The documentation for UnresolvedLookupExpr suggests that this is a bug when
parsing the template:

  /// These never include UnresolvedUsingValueDecls, which are always class
  /// members and therefore appear only in UnresolvedMemberLookupExprs.

(The comment presumably means UnresolvedMemberExpr.)

Later we have to notice that the decl resolved to
> an instance member and form a MemberExpr if so.
>
> http://reviews.llvm.org/D6700
>
> Files:
>   lib/Sema/SemaExprMember.cpp
>   lib/Sema/TreeTransform.h
>   test/SemaTemplate/instantiate-using-decl.cpp
>
> Index: lib/Sema/SemaExprMember.cpp
> ===================================================================
> --- lib/Sema/SemaExprMember.cpp
> +++ lib/Sema/SemaExprMember.cpp
> @@ -90,7 +90,6 @@
>  /// conservatively answer "yes", in which case some errors will simply
>  /// not be caught until template-instantiation.
>  static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
> -                                            Scope *CurScope,
>                                              const LookupResult &R) {
>    assert(!R.empty() && (*R.begin())->isCXXClassMember());
>
> @@ -205,6 +204,9 @@
>    SourceRange Range(Loc);
>    if (SS.isSet()) Range.setBegin(SS.getRange().getBegin());
>
> +  // Look through using shadow decls and aliases.
> +  Rep = Rep->getUnderlyingDecl();
>

You can test this change in isolation with:

struct A { int n; }; struct B : A { using A::n; static int f() { return n;
} };

... which currently produces this bogus diagnostic:

<stdin>:1:73: error: call to non-static member function without an object
argument
struct A { int n; }; struct B : A { using A::n; static int f() { return n;
} };
                                                                        ^

+
>    DeclContext *FunctionLevelDC = SemaRef.getFunctionLevelDeclContext();
>    CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FunctionLevelDC);
>    CXXRecordDecl *ContextClass = Method ? Method->getParent() : nullptr;
> @@ -237,7 +239,7 @@
>                                        SourceLocation TemplateKWLoc,
>                                        LookupResult &R,
>                                  const TemplateArgumentListInfo
> *TemplateArgs) {
> -  switch (ClassifyImplicitMemberAccess(*this, CurScope, R)) {
> +  switch (ClassifyImplicitMemberAccess(*this, R)) {
>    case IMA_Instance:
>      return BuildImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs,
> true);
>
> Index: lib/Sema/TreeTransform.h
> ===================================================================
> --- lib/Sema/TreeTransform.h
> +++ lib/Sema/TreeTransform.h
> @@ -8705,8 +8705,18 @@
>
>    // If we have neither explicit template arguments, nor the template
> keyword,
>    // it's a normal declaration name.
> -  if (!Old->hasExplicitTemplateArgs() && !TemplateKWLoc.isValid())
> +  if (!Old->hasExplicitTemplateArgs() && !TemplateKWLoc.isValid()) {
> +    // If this resolved to a data member, do member reference semantic
> analysis
> +    // and possibly form a MemberExpr.
> +    if (NamedDecl *D = R.getAsSingle<NamedDecl>()) {
> +      D = D->getUnderlyingDecl();
> +      if (isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D) ||
> +          isa<MSPropertyDecl>(D))
>

Per the comment on UnresolvedLookupExpr, I'm not sure this is the right
place for this fix. In any case, we have somewhat more complex logic for
this in ActOnIdExpression:

  if (!R.empty() && (*R.begin())->isCXXClassMember()) {
    bool MightBeImplicitMember;
    if (!IsAddressOfOperand)
      MightBeImplicitMember = true;
    else if (!SS.isEmpty())
      MightBeImplicitMember = false;
    else if (R.isOverloadedResult())
      MightBeImplicitMember = false;
    else if (R.isUnresolvableResult())
      MightBeImplicitMember = true;
    else
      MightBeImplicitMember = isa<FieldDecl>(R.getFoundDecl()) ||
                              isa<IndirectFieldDecl>(R.getFoundDecl()) ||
                              isa<MSPropertyDecl>(R.getFoundDecl());

    if (MightBeImplicitMember)
      return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
                                             R, TemplateArgs);
  }

At least some of those checks seem like they would be relevant here.

+        return getSema().BuildPossibleImplicitMemberExpr(
> +            SS, SourceLocation(), R, /*TemplateArgs=*/nullptr);
> +    }
>      return getDerived().RebuildDeclarationNameExpr(SS, R,
> Old->requiresADL());
> +  }
>
>    // If we have template arguments, rebuild them, then rebuild the
>    // templateid expression.
> Index: test/SemaTemplate/instantiate-using-decl.cpp
> ===================================================================
> --- test/SemaTemplate/instantiate-using-decl.cpp
> +++ test/SemaTemplate/instantiate-using-decl.cpp
> @@ -104,3 +104,26 @@
>      x.f();
>    }
>  }
> +
> +namespace PR21923 {
> +struct A {
> +  int member;
> +  void method();
> +};
> +template <typename T>
> +struct B : public T {
> +  using T::member;
> +  using T::method;
> +  static void StaticFun() {
> +    (void)member; // expected-error {{invalid use of member 'member' in
> static member function}}
> +    (void)B<T>::member; // expected-error {{invalid use of member
> 'member' in static member function}}
> +    method(); // expected-error {{call to non-static member function
> without an object argument}}
> +  }
> +  void InstanceFun() {
> +    (void)member;
> +    (void)B<T>::member;
> +    method();
> +  }
> +};
> +template class B<A>; // expected-note 1 {{requested here}}
> +}
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141216/0c1f3ec0/attachment.html>