george.burgess.iv created this revision. george.burgess.iv added a reviewer: rsmith. george.burgess.iv added a subscriber: cfe-commits.
Given the following declarations for `foo`: ``` void foo(int a); void foo(int a) __attribute__((enable_if(a, ""))); ``` ...The only way to `reinterpret_cast` `foo` is to insert two casts: ``` auto fnptr = reinterpret_cast<void (*)(void *)>((void (*)(int))foo); ``` ...Which isn't quite ideal. This patch teaches clang to check to see if an overload set has only one candidate that can have its address taken. If this is the case, then we'll use it automatically. (As a side note: we use a custom loop rather than modifying `AddressOfFunctionResolver` because we don't need to do any kind of ranking, and there seem to be some cases where `AddressOfFunctionResolver` will disqualify candidates, which we don't really want here. I can fold this logic into `AddressOfFunctionResolver` if we want to potentially have less code duplication, but I feel this is straightforward enough that it can stand on its own, rather than adding complexity to `AddressOfFunctionResolver`). http://reviews.llvm.org/D17701 Files: include/clang/Sema/Sema.h lib/Sema/SemaCast.cpp lib/Sema/SemaOverload.cpp test/SemaCXX/enable_if.cpp test/SemaCXX/unaddressable-functions.cpp
Index: test/SemaCXX/unaddressable-functions.cpp =================================================================== --- /dev/null +++ test/SemaCXX/unaddressable-functions.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 + +namespace access_control { +class Private { + void check(int *) __attribute__((enable_if(false, ""))); + void check(double *) __attribute__((enable_if(true, ""))); + + static void checkStatic(int *) __attribute__((enable_if(false, ""))); + static void checkStatic(double *) __attribute__((enable_if(true, ""))); +}; + +auto Priv = reinterpret_cast<void (Private::*)(char *)>(&Private::check); // expected-error{{'check' is a private member of 'access_control::Private'}} expected-note@6{{implicitly declared private here}} + +auto PrivStatic = reinterpret_cast<void (*)(char *)>(&Private::checkStatic); // expected-error{{'checkStatic' is a private member of 'access_control::Private'}} expected-note@9{{implicitly declared private here}} + +class Protected { +protected: + void check(int *) __attribute__((enable_if(false, ""))); + void check(double *) __attribute__((enable_if(true, ""))); + + static void checkStatic(int *) __attribute__((enable_if(false, ""))); + static void checkStatic(double *) __attribute__((enable_if(true, ""))); +}; + +auto Prot = reinterpret_cast<void (Protected::*)(char *)>(&Protected::check); // expected-error{{'check' is a protected member of 'access_control::Protected'}} expected-note@19{{declared protected here}} + +auto ProtStatic = reinterpret_cast<void (*)(char *)>(&Protected::checkStatic); // expected-error{{'checkStatic' is a protected member of 'access_control::Protected'}} expected-note@22{{declared protected here}} +} Index: test/SemaCXX/enable_if.cpp =================================================================== --- test/SemaCXX/enable_if.cpp +++ test/SemaCXX/enable_if.cpp @@ -253,3 +253,96 @@ a = &noOvlNoCandidate; // expected-error{{cannot take address of function 'noOvlNoCandidate' becuase it has one or more non-tautological enable_if conditions}} } } + +namespace casting { +using VoidFnTy = void (*)(); + +void foo(void *c) __attribute__((enable_if(0, ""))); +void foo(int *c) __attribute__((enable_if(c, ""))); +void foo(char *c) __attribute__((enable_if(1, ""))); + +void testIt() { + auto A = reinterpret_cast<VoidFnTy>(foo); + auto AAmp = reinterpret_cast<VoidFnTy>(&foo); + + using VoidFooTy = void (*)(void *); + auto B = reinterpret_cast<VoidFooTy>(foo); + auto BAmp = reinterpret_cast<VoidFooTy>(&foo); + + using IntFooTy = void (*)(int *); + auto C = reinterpret_cast<IntFooTy>(foo); + auto CAmp = reinterpret_cast<IntFooTy>(&foo); + + using CharFooTy = void (*)(void *); + auto D = reinterpret_cast<CharFooTy>(foo); + auto DAmp = reinterpret_cast<CharFooTy>(&foo); +} + +void testItCStyle() { + auto A = (VoidFnTy)foo; + auto AAmp = (VoidFnTy)&foo; + + using VoidFooTy = void (*)(void *); + auto B = (VoidFooTy)foo; + auto BAmp = (VoidFooTy)&foo; + + using IntFooTy = void (*)(int *); + auto C = (IntFooTy)foo; + auto CAmp = (IntFooTy)&foo; + + using CharFooTy = void (*)(void *); + auto D = (CharFooTy)foo; + auto DAmp = (CharFooTy)&foo; +} +} + +namespace casting_templates { +template <typename T> void foo(T) {} // expected-note 4 {{candidate function}} + +void foo(int *c) __attribute__((enable_if(c, ""))); //expected-note 4 {{candidate function}} +void foo(char *c) __attribute__((enable_if(c, ""))); //expected-note 4 {{candidate function}} + +void testIt() { + using IntFooTy = void (*)(int *); + auto A = reinterpret_cast<IntFooTy>(foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + auto ARef = reinterpret_cast<IntFooTy>(&foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + auto AExplicit = reinterpret_cast<IntFooTy>(foo<int*>); + + using CharFooTy = void (*)(char *); + auto B = reinterpret_cast<CharFooTy>(foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + auto BRef = reinterpret_cast<CharFooTy>(&foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + auto BExplicit = reinterpret_cast<CharFooTy>(foo<char*>); +} + +void testItCStyle() { + // constexpr is usable here because all of these should become static_casts. + using IntFooTy = void (*)(int *); + constexpr auto A = (IntFooTy)foo; + constexpr auto ARef = (IntFooTy)&foo; + constexpr auto AExplicit = (IntFooTy)foo<int*>; + + using CharFooTy = void (*)(char *); + constexpr auto B = (CharFooTy)foo; + constexpr auto BRef = (CharFooTy)&foo; + constexpr auto BExplicit = (CharFooTy)foo<char*>; + + static_assert(A == ARef && ARef == AExplicit, ""); + static_assert(B == BRef && BRef == BExplicit, ""); +} +} + +namespace multiple_matches { +using NoMatchTy = void (*)(); + +void foo(float *c); //expected-note 4 {{candidate function}} +void foo(int *c) __attribute__((enable_if(1, ""))); //expected-note 4 {{candidate function}} +void foo(char *c) __attribute__((enable_if(1, ""))); //expected-note 4 {{candidate function}} + +void testIt() { + auto A = reinterpret_cast<NoMatchTy>(foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + auto ARef = reinterpret_cast<NoMatchTy>(&foo); // expected-error{{reinterpret_cast cannot resolve overloaded function 'foo' to type}} + + auto C = (NoMatchTy)foo; // expected-error{{address of overloaded function 'foo' does not match required type 'void ()'}} + auto CRef = (NoMatchTy)&foo; // expected-error{{address of overloaded function 'foo' does not match required type 'void ()'}} +} +} Index: lib/Sema/SemaOverload.cpp =================================================================== --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -10412,7 +10412,7 @@ return false; QualType ResultTy; - if (Context.hasSameUnqualifiedType(TargetFunctionType, + if (Context.hasSameUnqualifiedType(TargetFunctionType, FunDecl->getType()) || S.IsNoReturnConversion(FunDecl->getType(), TargetFunctionType, ResultTy) || @@ -10645,6 +10645,42 @@ } /// \brief Given an expression that refers to an overloaded function, try to +/// resolve that function to a single function that can have its address taken. +/// This will modify `Pair` iff it returns non-null. +/// +/// This routine can only realistically succeed if all but one candidates in the +/// overload set for SrcExpr cannot have their addresses taken. +FunctionDecl * +Sema::resolveAddressOfOnlyViableOverloadCandidate(Expr *E, + DeclAccessPair &Pair) { + OverloadExpr::FindResult R = OverloadExpr::find(E); + OverloadExpr *Ovl = R.Expression; + FunctionDecl *Result = nullptr; + DeclAccessPair DAP; + // Don't use the AddressOfResolver because we're specifically looking for + // cases where we have one overload candidate that lacks + // enable_if/pass_object_size/... + for (auto I = Ovl->decls_begin(), E = Ovl->decls_end(); I != E; ++I) { + auto *FD = dyn_cast<FunctionDecl>(I->getUnderlyingDecl()); + if (!FD) + return nullptr; + + if (!checkAddressOfFunctionIsAvailable(FD)) + continue; + + // We have more than one result; quit. + if (Result) + return nullptr; + DAP = I.getPair(); + Result = FD; + } + + if (Result) + Pair = DAP; + return Result; +} + +/// \brief Given an expression that refers to an overloaded function, try to /// resolve that overloaded function expression down to a single function. /// /// This routine can only resolve template-ids that refer to a single function Index: lib/Sema/SemaCast.cpp =================================================================== --- lib/Sema/SemaCast.cpp +++ lib/Sema/SemaCast.cpp @@ -1750,6 +1750,40 @@ } } +static bool fixOverloadedReinterpretCastExpr(Sema &Self, QualType DestType, + ExprResult &Result) { + // We can only fix an overloaded reinterpret_cast if + // - it is a template with explicit arguments that resolves to an lvalue + // unambiguously, or + // - it is the only function in an overload set that may have its address + // taken. + + Expr *E = Result.get(); + // TODO: what if this fails because of DiagnoseUseOfDecl or something + // like it? + if (Self.ResolveAndFixSingleFunctionTemplateSpecialization( + Result, + Expr::getValueKindForType(DestType) == VK_RValue // Convert Fun to Ptr + ) && + Result.isUsable()) + return true; + + DeclAccessPair DAP; + FunctionDecl *Found = Self.resolveAddressOfOnlyViableOverloadCandidate(E, DAP); + if (!Found) + return false; + + Self.CheckAddressOfMemberAccess(E, DAP); + + Expr *Fixed = Self.FixOverloadedFunctionReference(E, DAP, Found); + if (Fixed->getType()->isFunctionType()) + Result = Self.DefaultFunctionArrayConversion(Fixed, /*Diagnose=*/false); + else + Result = Fixed; + + return !Result.isInvalid(); +} + static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr, QualType DestType, bool CStyle, SourceRange OpRange, @@ -1761,21 +1795,15 @@ QualType SrcType = SrcExpr.get()->getType(); // Is the source an overloaded name? (i.e. &foo) - // If so, reinterpret_cast can not help us here (13.4, p1, bullet 5) ... + // If so, reinterpret_cast generally can not help us here (13.4, p1, bullet 5) if (SrcType == Self.Context.OverloadTy) { - // ... unless foo<int> resolves to an lvalue unambiguously. - // TODO: what if this fails because of DiagnoseUseOfDecl or something - // like it? - ExprResult SingleFunctionExpr = SrcExpr; - if (Self.ResolveAndFixSingleFunctionTemplateSpecialization( - SingleFunctionExpr, - Expr::getValueKindForType(DestType) == VK_RValue // Convert Fun to Ptr - ) && SingleFunctionExpr.isUsable()) { - SrcExpr = SingleFunctionExpr; - SrcType = SrcExpr.get()->getType(); - } else { + ExprResult FixedExpr = SrcExpr; + if (!fixOverloadedReinterpretCastExpr(Self, DestType, FixedExpr)) return TC_NotApplicable; - } + + assert(FixedExpr.isUsable() && "Invalid result fixing overloaded expr"); + SrcExpr = FixedExpr; + SrcType = SrcExpr.get()->getType(); } if (const ReferenceType *DestTypeTmp = DestType->getAs<ReferenceType>()) { Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -2505,6 +2505,10 @@ bool *pHadMultipleCandidates = nullptr); FunctionDecl * + resolveAddressOfOnlyViableOverloadCandidate(Expr *E, + DeclAccessPair &FoundResult); + + FunctionDecl * ResolveSingleFunctionTemplateSpecialization(OverloadExpr *ovl, bool Complain = false, DeclAccessPair *Found = nullptr);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits