From 9c72c6088d591ace8503b842d39448c2040f3033 Mon Sep 17 00:00:00 2001 From: John McCall Date: Fri, 27 Aug 2010 09:08:28 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@112258 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ExprCXX.h | 33 ++++++-- include/clang/Basic/DiagnosticSemaKinds.td | 6 +- include/clang/Parse/Parser.h | 10 +++ include/clang/Sema/Sema.h | 2 +- lib/AST/ExprClassification.cpp | 4 + lib/Parse/ParseExpr.cpp | 6 +- lib/Parse/ParseExprCXX.cpp | 17 +--- lib/Sema/SemaAccess.cpp | 2 +- lib/Sema/SemaExpr.cpp | 98 ++++++++++++++-------- lib/Sema/SemaOverload.cpp | 18 +--- lib/Sema/SemaTemplateDeduction.cpp | 25 ++++-- test/SemaCXX/member-pointer.cpp | 67 ++++++++++++++- 12 files changed, 200 insertions(+), 88 deletions(-) diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h index 418bb26e5b..450348b9d2 100644 --- a/include/clang/AST/ExprCXX.h +++ b/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 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(E)) - op = true, E = cast(E)->getSubExpr()->IgnoreParens(); - return llvm::PointerIntPair(cast(E), op); + if (isa(E)) { + assert(cast(E)->getOpcode() == UO_AddrOf); + E = cast(E)->getSubExpr(); + OverloadExpr *Ovl = cast(E->IgnoreParens()); + + Result.HasFormOfMemberPointer = (E == Ovl && Ovl->getQualifier()); + Result.IsAddressOfOperand = true; + Result.Expression = Ovl; + } else { + Result.HasFormOfMemberPointer = false; + Result.IsAddressOfOperand = false; + Result.Expression = cast(E); + } + + return Result; } /// Gets the naming class of this lookup, if any. diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 99dfa87133..2a1e8a7a29 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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< diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 5a8ff91cb1..1116c956a4 100644 --- a/include/clang/Parse/Parser.h +++ b/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(); diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index fbc15d4beb..b1eb2b2adf 100644 --- a/include/clang/Sema/Sema.h +++ b/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, diff --git a/lib/AST/ExprClassification.cpp b/lib/AST/ExprClassification.cpp index c54dcb60b9..d7e38ebbf5 100644 --- a/lib/AST/ExprClassification.cpp +++ b/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(D) && cast(D)->isInstance()) + return Cl::CL_MemberFunction; + bool islvalue; if (const NonTypeTemplateParmDecl *NTTParm = dyn_cast(D)) diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index d70eb63a78..290b72c4c0 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/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 diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index 7270f6a909..5041a21292 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/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); diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp index 7cda7dd172..758682f164 100644 --- a/lib/Sema/SemaAccess.cpp +++ b/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, diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index cbb7842ad5..21c7e44544 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/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(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(op); - if (lval == Expr::LV_MemberFunction && ME && - isa(ME->getMemberDecl())) { - ValueDecl *dcl = cast(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(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(op)) + } else if (isa(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(op)) { + Diag(OpLoc, diag::err_invalid_form_pointer_member_function) + << OrigOp->getSourceRange(); + return QualType(); + } + DeclRefExpr *DRE = cast(op); + CXXMethodDecl *MD = cast(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(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(Ctx)).getTypePtr()); } } - } else if (CXXMethodDecl *MD = dyn_cast(dcl)) { - // Okay: we can take the address of a function. - // As above. - if (isa(op) && cast(op)->getQualifier() && - MD->isInstance()) - return Context.getMemberPointerType(op->getType(), - Context.getTypeDeclType(MD->getParent()).getTypePtr()); } else if (!isa(dcl)) assert(0 && "Unknown/unexpected decl type"); } diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 95c03eb768..b5f1e528e4 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/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(Tmp)) { - Tmp = cast(Tmp)->getSubExpr(); - OvlExpr = cast(Tmp->IgnoreParens()); - HasFormOfMemberPointer = (Tmp == OvlExpr && OvlExpr->getQualifier()); - } else { - OvlExpr = cast(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()) diff --git a/lib/Sema/SemaTemplateDeduction.cpp b/lib/Sema/SemaTemplateDeduction.cpp index 620f7d53f9..4691181111 100644 --- a/lib/Sema/SemaTemplateDeduction.cpp +++ b/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(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 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(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 diff --git a/test/SemaCXX/member-pointer.cpp b/test/SemaCXX/member-pointer.cpp index 9d5cd2fc92..83823f69c6 100644 --- a/test/SemaCXX/member-pointer.cpp +++ b/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 void bind(F f); // expected-note 6 {{candidate template ignored}} + template void bindmem(F (T::*f)()); // expected-note 4 {{candidate template ignored}} + template 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 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 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 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); + } + }; + }; +} -- 2.40.0