Propagate whether an id-expression is the immediate argument of

an '&' expression from the second caller of ActOnIdExpression.

Teach template argument deduction that an overloaded id-expression
doesn't give a valid type for deduction purposes to a non-static
member function unless the expression has the correct syntactic
form.

Teach ActOnIdExpression that it shouldn't try to create implicit
member expressions for '&function', because this isn't a        
permitted form of use for member functions.

Teach CheckAddressOfOperand to diagnose these more carefully.
Some of these cases aren't reachable right now because earlier
diagnostics interrupt them.

llvm-svn: 112258

clang/include/clang/AST/ExprCXX.h

@@ -1440,19 +1440,38 @@ public:
                                 UnresolvedSetIterator End,
                                 const TemplateArgumentListInfo *Args);
 
+  struct FindResult {
+    OverloadExpr *Expression;
+    bool IsAddressOfOperand;
+    bool HasFormOfMemberPointer;
+  };
+
   /// Finds the overloaded expression in the given expression of
   /// OverloadTy.
   ///
-  /// \return the expression (which must be there) and true if it is
-  /// within an address-of operator.
-  static llvm::PointerIntPair<OverloadExpr*,1> find(Expr *E) {
+  /// \return the expression (which must be there) and true if it has
+  /// the particular form of a member pointer expression
+  static FindResult find(Expr *E) {
     assert(E->getType()->isSpecificBuiltinType(BuiltinType::Overload));
 
-    bool op = false;
+    FindResult Result;
+
     E = E->IgnoreParens();
-    if (isa<UnaryOperator>(E))
-      op = true, E = cast<UnaryOperator>(E)->getSubExpr()->IgnoreParens();
-    return llvm::PointerIntPair<OverloadExpr*,1>(cast<OverloadExpr>(E), op);
+    if (isa<UnaryOperator>(E)) {
+      assert(cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf);
+      E = cast<UnaryOperator>(E)->getSubExpr();
+      OverloadExpr *Ovl = cast<OverloadExpr>(E->IgnoreParens());
+
+      Result.HasFormOfMemberPointer = (E == Ovl && Ovl->getQualifier());
+      Result.IsAddressOfOperand = true;
+      Result.Expression = Ovl;
+    } else {
+      Result.HasFormOfMemberPointer = false;
+      Result.IsAddressOfOperand = false;
+      Result.Expression = cast<OverloadExpr>(E);
+    }
+
+    return Result;
   }
 
   /// Gets the naming class of this lookup, if any.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2127,7 +2127,11 @@ def err_typecheck_address_of : Error<"address of %0 requested">;
 def ext_typecheck_addrof_void : Extension<
   "ISO C forbids taking the address of an expression of type 'void'">;
 def err_unqualified_pointer_member_function : Error<
-  "must explicitly qualify member function %0 when taking its address">;
+  "must explicitly qualify name of member function when taking its address">;
+def err_invalid_form_pointer_member_function : Error<
+  "cannot create a non-constant pointer to member function">;
+def err_parens_pointer_member_function : Error<
+  "cannot parenthesize the name of a method when forming a member pointer">;
 def err_typecheck_invalid_lvalue_addrof : Error<
   "address expression must be an lvalue or a function designator">;
 def ext_typecheck_addrof_class_temporary : ExtWarn<

clang/include/clang/Parse/Parser.h

@@ -949,6 +949,16 @@ private:
   ExprResult ParseCastExpression(bool isUnaryExpression,
                                        bool isAddressOfOperand = false,
                                        ParsedType TypeOfCast = ParsedType());
+
+  /// Returns true if the next token would start a postfix-expression
+  /// suffix.
+  bool isPostfixExpressionSuffixStart() {
+    tok::TokenKind K = Tok.getKind();
+    return (K == tok::l_square || K == tok::l_paren ||
+            K == tok::period || K == tok::arrow ||
+            K == tok::plusplus || K == tok::minusminus);
+  }
+
   ExprResult ParsePostfixExpressionSuffix(ExprResult LHS);
   ExprResult ParseSizeofAlignofExpression();
   ExprResult ParseBuiltinPrimaryExpression();

clang/include/clang/Sema/Sema.h

@@ -1715,7 +1715,7 @@ public:
                                            Expr *BaseObjectExpr = 0,
                                       SourceLocation OpLoc = SourceLocation());
   ExprResult BuildPossibleImplicitMemberExpr(const CXXScopeSpec &SS,
-                                                   LookupResult &R,
+                                             LookupResult &R,
                                 const TemplateArgumentListInfo *TemplateArgs);
   ExprResult BuildImplicitMemberExpr(const CXXScopeSpec &SS,
                                            LookupResult &R,

clang/lib/AST/ExprClassification.cpp

@@ -229,6 +229,10 @@ static Cl::Kinds ClassifyDecl(ASTContext &Ctx, const Decl *D) {
   // In addition, NonTypeTemplateParmDecl derives from VarDecl but isn't an
   // lvalue unless it's a reference type (C++ [temp.param]p6), so we need to
   // special-case this.
+
+  if (isa<CXXMethodDecl>(D) && cast<CXXMethodDecl>(D)->isInstance())
+    return Cl::CL_MemberFunction;
+
   bool islvalue;
   if (const NonTypeTemplateParmDecl *NTTParm =
         dyn_cast<NonTypeTemplateParmDecl>(D))

clang/lib/Parse/ParseExpr.cpp

@@ -660,6 +660,10 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
                                               ILoc, PropertyLoc);
       break;
     }
+
+    // Make sure to pass down the right value for isAddressOfOperand.
+    if (isAddressOfOperand && isPostfixExpressionSuffixStart())
+      isAddressOfOperand = false;
    
     // Function designators are allowed to be undeclared (C99 6.5.1p2), so we
     // need to know whether or not this identifier is a function designator or
@@ -668,7 +672,7 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
     CXXScopeSpec ScopeSpec;
     Name.setIdentifier(&II, ILoc);
     Res = Actions.ActOnIdExpression(getCurScope(), ScopeSpec, Name, 
-                                    Tok.is(tok::l_paren), false);
+                                    Tok.is(tok::l_paren), isAddressOfOperand);
     break;
   }
   case tok::char_constant:     // constant: character-constant

clang/lib/Parse/ParseExprCXX.cpp

@@ -416,21 +416,8 @@ ExprResult Parser::ParseCXXIdExpression(bool isAddressOfOperand) {
 
   // This is only the direct operand of an & operator if it is not
   // followed by a postfix-expression suffix.
-  if (isAddressOfOperand) {
-    switch (Tok.getKind()) {
-    case tok::l_square:
-    case tok::l_paren:
-    case tok::arrow:
-    case tok::period:
-    case tok::plusplus:
-    case tok::minusminus:
-      isAddressOfOperand = false;
-      break;
-
-    default:
-      break;
-    }
-  }
+  if (isAddressOfOperand && isPostfixExpressionSuffixStart())
+    isAddressOfOperand = false;
   
   return Actions.ActOnIdExpression(getCurScope(), SS, Name, Tok.is(tok::l_paren),
                                    isAddressOfOperand);

clang/lib/Sema/SemaAccess.cpp

@@ -1268,7 +1268,7 @@ Sema::AccessResult Sema::CheckAddressOfMemberAccess(Expr *OvlExpr,
       Found.getAccess() == AS_public)
     return AR_accessible;
 
-  OverloadExpr *Ovl = OverloadExpr::find(OvlExpr).getPointer();
+  OverloadExpr *Ovl = OverloadExpr::find(OvlExpr).Expression;
   CXXRecordDecl *NamingClass = Ovl->getNamingClass();
 
   AccessTarget Entity(Context, AccessTarget::Member, NamingClass, Found,

clang/lib/Sema/SemaExpr.cpp

@@ -1233,11 +1233,33 @@ ExprResult Sema::ActOnIdExpression(Scope *S,
   //   member of some class C, the id-expression is transformed into a
   //   class member access expression using (*this) as the
   //   postfix-expression to the left of the . operator.
-  // So if we found a class member with an expression of form other
-  // than &A::foo, we have to try to build an implicit member expr.
+  //
+  // But we don't actually need to do this for '&' operands if R
+  // resolved to a function or overloaded function set, because the
+  // expression is ill-formed if it actually works out to be a
+  // non-static member function:
+  //
+  // C++ [expr.ref]p4:
+  //   Otherwise, if E1.E2 refers to a non-static member function. . .
+  //   [t]he expression can be used only as the left-hand operand of a
+  //   member function call.
+  //
+  // There are other safeguards against such uses, but it's important
+  // to get this right here so that we don't end up making a
+  // spuriously dependent expression if we're inside a dependent
+  // instance method.
   if (!R.empty() && (*R.begin())->isCXXClassMember()) {
-    bool isAbstractMemberPointer = (isAddressOfOperand && !SS.isEmpty());
-    if (!isAbstractMemberPointer)
+    bool MightBeImplicitMember;
+    if (!isAddressOfOperand)
+      MightBeImplicitMember = true;
+    else if (!SS.isEmpty())
+      MightBeImplicitMember = false;
+    else if (R.isOverloadedResult())
+      MightBeImplicitMember = false;
+    else
+      MightBeImplicitMember = isa<FieldDecl>(R.getFoundDecl());
+
+    if (MightBeImplicitMember)
       return BuildPossibleImplicitMemberExpr(SS, R, TemplateArgs);
   }
 
@@ -6209,12 +6231,14 @@ static NamedDecl *getPrimaryDecl(Expr *E) {
 /// operator (C99 6.3.2.1p[2-4]), and its result is never an lvalue.
 /// In C++, the operand might be an overloaded function name, in which case
 /// we allow the '&' but retain the overloaded-function type.
-QualType Sema::CheckAddressOfOperand(Expr *op, SourceLocation OpLoc) {
-  // Make sure to ignore parentheses in subsequent checks
-  op = op->IgnoreParens();
-
-  if (op->isTypeDependent())
+QualType Sema::CheckAddressOfOperand(Expr *OrigOp, SourceLocation OpLoc) {
+  if (OrigOp->isTypeDependent())
     return Context.DependentTy;
+  if (OrigOp->getType() == Context.OverloadTy)
+    return Context.OverloadTy;
+
+  // Make sure to ignore parentheses in subsequent checks
+  Expr *op = OrigOp->IgnoreParens();
 
   if (getLangOptions().C99) {
     // Implement C99-only parts of addressof rules.
@@ -6230,32 +6254,41 @@ QualType Sema::CheckAddressOfOperand(Expr *op, SourceLocation OpLoc) {
   NamedDecl *dcl = getPrimaryDecl(op);
   Expr::isLvalueResult lval = op->isLvalue(Context);
 
-  MemberExpr *ME = dyn_cast<MemberExpr>(op);
-  if (lval == Expr::LV_MemberFunction && ME &&
-      isa<CXXMethodDecl>(ME->getMemberDecl())) {
-    ValueDecl *dcl = cast<MemberExpr>(op)->getMemberDecl();
-    // &f where f is a member of the current object, or &o.f, or &p->f
-    // All these are not allowed, and we need to catch them before the dcl
-    // branch of the if, below.
-    Diag(OpLoc, diag::err_unqualified_pointer_member_function)
-        << dcl;
-    // FIXME: Improve this diagnostic and provide a fixit.
-
-    // Now recover by acting as if the function had been accessed qualified.
-    return Context.getMemberPointerType(op->getType(),
-                Context.getTypeDeclType(cast<RecordDecl>(dcl->getDeclContext()))
-                       .getTypePtr());
-  }
-  
   if (lval == Expr::LV_ClassTemporary) {
     Diag(OpLoc, isSFINAEContext()? diag::err_typecheck_addrof_class_temporary
                                  : diag::ext_typecheck_addrof_class_temporary)
       << op->getType() << op->getSourceRange();
     if (isSFINAEContext())
       return QualType();
-  } else if (isa<ObjCSelectorExpr>(op))
+  } else if (isa<ObjCSelectorExpr>(op)) {
     return Context.getPointerType(op->getType());
-  else if (lval != Expr::LV_Valid && lval != Expr::LV_IncompleteVoidType) {
+  } else if (lval == Expr::LV_MemberFunction) {
+    // If it's an instance method, make a member pointer.
+    // The expression must have exactly the form &A::foo.
+
+    // If the underlying expression isn't a decl ref, give up.
+    if (!isa<DeclRefExpr>(op)) {
+      Diag(OpLoc, diag::err_invalid_form_pointer_member_function)
+        << OrigOp->getSourceRange();
+      return QualType();
+    }
+    DeclRefExpr *DRE = cast<DeclRefExpr>(op);
+    CXXMethodDecl *MD = cast<CXXMethodDecl>(DRE->getDecl());
+
+    // The id-expression was parenthesized.
+    if (OrigOp != DRE) {
+      Diag(OpLoc, diag::err_parens_pointer_member_function)
+        << OrigOp->getSourceRange();
+
+    // The method was named without a qualifier.
+    } else if (!DRE->getQualifier()) {
+      Diag(OpLoc, diag::err_unqualified_pointer_member_function)
+        << op->getSourceRange();
+    }
+
+    return Context.getMemberPointerType(op->getType(),
+              Context.getTypeDeclType(MD->getParent()).getTypePtr());
+  } else if (lval != Expr::LV_Valid && lval != Expr::LV_IncompleteVoidType) {
     // C99 6.5.3.2p1
     // The operand must be either an l-value or a function designator
     if (!op->getType()->isFunctionType()) {
@@ -6283,8 +6316,6 @@ QualType Sema::CheckAddressOfOperand(Expr *op, SourceLocation OpLoc) {
     // FIXME: Can LHS ever be null here?
     if (!CheckAddressOfOperand(CO->getTrueExpr(), OpLoc).isNull())
       return CheckAddressOfOperand(CO->getFalseExpr(), OpLoc);
-  } else if (isa<OverloadExpr>(op)) {
-    return Context.OverloadTy;
   } else if (dcl) { // C99 6.5.3.2p1
     // We have an lvalue with a decl. Make sure the decl is not declared
     // with the register storage-class specifier.
@@ -6317,13 +6348,6 @@ QualType Sema::CheckAddressOfOperand(Expr *op, SourceLocation OpLoc) {
                 Context.getTypeDeclType(cast<RecordDecl>(Ctx)).getTypePtr());
         }
       }
-    } else if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(dcl)) {
-      // Okay: we can take the address of a function.
-      // As above.
-      if (isa<DeclRefExpr>(op) && cast<DeclRefExpr>(op)->getQualifier() &&
-          MD->isInstance())
-        return Context.getMemberPointerType(op->getType(),
-              Context.getTypeDeclType(MD->getParent()).getTypePtr());
     } else if (!isa<FunctionDecl>(dcl))
       assert(0 && "Unknown/unexpected decl type");
   }

clang/lib/Sema/SemaOverload.cpp

@@ -6222,18 +6222,8 @@ Sema::ResolveAddressOfOverloadedFunction(Expr *From, QualType ToType,
   //     A pointer to member is only formed when an explicit & is used
   //     and its operand is a qualified-id not enclosed in
   //     parentheses.
-  bool HasFormOfMemberPointer = false;
-  OverloadExpr *OvlExpr;
-  {
-    Expr *Tmp = From->IgnoreParens();
-    if (isa<UnaryOperator>(Tmp)) {
-      Tmp = cast<UnaryOperator>(Tmp)->getSubExpr();
-      OvlExpr = cast<OverloadExpr>(Tmp->IgnoreParens());
-      HasFormOfMemberPointer = (Tmp == OvlExpr && OvlExpr->getQualifier());
-    } else {
-      OvlExpr = cast<OverloadExpr>(Tmp);
-    }
-  }
+  OverloadExpr::FindResult Ovl = OverloadExpr::find(From);
+  OverloadExpr *OvlExpr = Ovl.Expression;
   
   // We expect a pointer or reference to function, or a function pointer.
   FunctionType = Context.getCanonicalType(FunctionType).getUnqualifiedType();
@@ -6247,7 +6237,7 @@ Sema::ResolveAddressOfOverloadedFunction(Expr *From, QualType ToType,
 
   // If the overload expression doesn't have the form of a pointer to
   // member, don't try to convert it to a pointer-to-member type.
-  if (IsMember && !HasFormOfMemberPointer) {
+  if (IsMember && !Ovl.HasFormOfMemberPointer) {
     if (!Complain) return 0;
 
     // TODO: Should we condition this on whether any functions might
@@ -6453,7 +6443,7 @@ FunctionDecl *Sema::ResolveSingleFunctionTemplateSpecialization(Expr *From) {
   if (From->getType() != Context.OverloadTy)
     return 0;
 
-  OverloadExpr *OvlExpr = OverloadExpr::find(From).getPointer();
+  OverloadExpr *OvlExpr = OverloadExpr::find(From).Expression;
   
   // If we didn't actually find any template-ids, we're done.
   if (!OvlExpr->hasExplicitTemplateArgs())

clang/lib/Sema/SemaTemplateDeduction.cpp

@@ -1494,14 +1494,22 @@ Sema::FinishTemplateArgumentDeduction(FunctionTemplateDecl *FunctionTemplate,
   return TDK_Success;
 }
 
+/// Gets the type of a function for template-argument-deducton
+/// purposes when it's considered as part of an overload set.
 static QualType GetTypeOfFunction(ASTContext &Context,
-                                  bool isAddressOfOperand,
+                                  const OverloadExpr::FindResult &R,
                                   FunctionDecl *Fn) {
-  if (!isAddressOfOperand) return Fn->getType();
   if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Fn))
-    if (Method->isInstance())
+    if (Method->isInstance()) {
+      // An instance method that's referenced in a form that doesn't
+      // look like a member pointer is just invalid.
+      if (!R.HasFormOfMemberPointer) return QualType();
+
       return Context.getMemberPointerType(Fn->getType(),
                Context.getTypeDeclType(Method->getParent()).getTypePtr());
+    }
+
+  if (!R.IsAddressOfOperand) return Fn->getType();
   return Context.getPointerType(Fn->getType());
 }
 
@@ -1512,10 +1520,10 @@ static QualType GetTypeOfFunction(ASTContext &Context,
 static QualType
 ResolveOverloadForDeduction(Sema &S, TemplateParameterList *TemplateParams,
                             Expr *Arg, QualType ParamType) {
-  llvm::PointerIntPair<OverloadExpr*,1> R = OverloadExpr::find(Arg);
+  
+  OverloadExpr::FindResult R = OverloadExpr::find(Arg);
 
-  bool isAddressOfOperand = bool(R.getInt());
-  OverloadExpr *Ovl = R.getPointer();
+  OverloadExpr *Ovl = R.Expression;
 
   // If there were explicit template arguments, we can only find
   // something via C++ [temp.arg.explicit]p3, i.e. if the arguments
@@ -1524,7 +1532,7 @@ ResolveOverloadForDeduction(Sema &S, TemplateParameterList *TemplateParams,
     // But we can still look for an explicit specialization.
     if (FunctionDecl *ExplicitSpec
           = S.ResolveSingleFunctionTemplateSpecialization(Ovl))
-      return GetTypeOfFunction(S.Context, isAddressOfOperand, ExplicitSpec);
+      return GetTypeOfFunction(S.Context, R, ExplicitSpec);
     return QualType();
   }
 
@@ -1549,7 +1557,8 @@ ResolveOverloadForDeduction(Sema &S, TemplateParameterList *TemplateParams,
       return QualType();
 
     FunctionDecl *Fn = cast<FunctionDecl>(D);
-    QualType ArgType = GetTypeOfFunction(S.Context, isAddressOfOperand, Fn);
+    QualType ArgType = GetTypeOfFunction(S.Context, R, Fn);
+    if (ArgType.isNull()) continue;
 
     //   - If the argument is an overload set (not containing function
     //     templates), trial argument deduction is attempted using each

clang/test/SemaCXX/member-pointer.cpp

@@ -79,7 +79,7 @@ void g() {
 
   void (HasMembers::*pmf)() = &HasMembers::f;
   void (*pnf)() = &Fake::f;
-  &hm.f; // expected-error {{must explicitly qualify}} expected-warning{{result unused}}
+  &hm.f; // expected-error {{cannot create a non-constant pointer to member function}}
 
   void (HasMembers::*pmgv)() = &HasMembers::g;
   void (HasMembers::*pmgi)(int) = &HasMembers::g;
@@ -142,8 +142,8 @@ namespace pr5985 {
     void f() {
       void (c::*p)();
       p = &h; // expected-error {{must explicitly qualify}}
-      p = &this->h; // expected-error {{must explicitly qualify}}
-      p = &(*this).h; // expected-error {{must explicitly qualify}}
+      p = &this->h; // expected-error {{cannot create a non-constant pointer to member function}}
+      p = &(*this).h; // expected-error {{cannot create a non-constant pointer to member function}}
     }
   };
 }
@@ -174,3 +174,64 @@ namespace PR7176 {
   void m()
   { (void)(Condition) &base::Continuous::cond; }
 }
+
+namespace rdar8358512 {
+  // We can't call this with an overload set because we're not allowed
+  // to look into overload sets unless the parameter has some kind of
+  // function type.
+  template <class F> void bind(F f); // expected-note 6 {{candidate template ignored}}
+  template <class F, class T> void bindmem(F (T::*f)()); // expected-note 4 {{candidate template ignored}}
+  template <class F> void bindfn(F (*f)()); // expected-note 4 {{candidate template ignored}}
+
+  struct A {
+    void member();
+
+    void nonstat();
+    void nonstat(int);
+
+    void mixed();
+    static void mixed(int);
+
+    static void stat();
+    static void stat(int);
+    
+    template <typename T> struct Test0 {
+      void test() {
+        bind(&nonstat); // expected-error {{no matching function for call}}
+        bind(&A::nonstat); // expected-error {{no matching function for call}}
+
+        bind(&mixed); // expected-error {{no matching function for call}}
+        bind(&A::mixed); // expected-error {{no matching function for call}}
+
+        bind(&stat); // expected-error {{no matching function for call}}
+        bind(&A::stat); // expected-error {{no matching function for call}}
+      }
+    };
+
+    template <typename T> struct Test1 {
+      void test() {
+        bindmem(&nonstat); // expected-error {{no matching function for call}}
+        bindmem(&A::nonstat);
+
+        bindmem(&mixed); // expected-error {{no matching function for call}}
+        bindmem(&A::mixed);
+
+        bindmem(&stat); // expected-error {{no matching function for call}}
+        bindmem(&A::stat); // expected-error {{no matching function for call}}
+      }
+    };
+
+    template <typename T> struct Test2 {
+      void test() {
+        bindfn(&nonstat); // expected-error {{no matching function for call}}
+        bindfn(&A::nonstat); // expected-error {{no matching function for call}}
+
+        bindfn(&mixed); // expected-error {{no matching function for call}}
+        bindfn(&A::mixed); // expected-error {{no matching function for call}}
+
+        bindfn(&stat);
+        bindfn(&A::stat);
+      }
+    };
+  };
+}