[Sema] Teach SemaCast to allow reinterpret_casts of overloaded functions with only one addressable candidate

[george.burgess.iv]

Summary

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).

[rsmith]

OK, though I'm hoping either there aren't many more of these cases or we can treat them more generally somehow.

lib/Sema/SemaCast.cpp

1771–1782 ↗

(On Diff #49334)

Do we reach a DiagnoseUseOfDecl somewhere in here?

[george.burgess.iv]

lib/Sema/SemaCast.cpp

1771–1782 ↗

(On Diff #49334)

Good catch.

On a slightly related side note, it seems that ResolveAndFixSingleFunctionTemplateSpecialization doesn't check for member access (though it does call DiagnoseUseOfDecl). Will commit a fix for that separately. :)

cfe/trunk/include/clang/Sema/Sema.h

   FunctionDecl *
   ResolveAddressOfOverloadedFunction(Expr *AddressOfExpr,
                                      QualType TargetType,
                                      bool Complain,
                                      DeclAccessPair &Found,
                                      bool *pHadMultipleCandidates = nullptr);
 
   FunctionDecl *
+  resolveAddressOfOnlyViableOverloadCandidate(Expr *E,
+                                              DeclAccessPair &FoundResult);
+
+  FunctionDecl *
   ResolveSingleFunctionTemplateSpecialization(OverloadExpr *ovl,
                                               bool Complain = false,
                                               DeclAccessPair *Found = nullptr);
 
   bool ResolveAndFixSingleFunctionTemplateSpecialization(
                       ExprResult &SrcExpr,
                       bool DoFunctionPointerConverion = false,
                       bool Complain = false,

cfe/trunk/lib/Sema/SemaCast.cpp

if (CStyle && SrcType->isIntegralType(Self.Context)
     // for their workflow.
     unsigned Diag = DestType->isVoidPointerType() ?
                       diag::warn_int_to_void_pointer_cast
                     : diag::warn_int_to_pointer_cast;
     Self.Diag(Loc, Diag) << SrcType << DestType;
   }
 }
 
+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;
+
+  // It seems that if we encounter a call to a function that is both unavailable
+  // and inaccessible, we'll emit multiple diags for said call. Hence, we run
+  // both checks below unconditionally.
+  Self.DiagnoseUseOfDecl(Found, E->getExprLoc());
+  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,
                                         unsigned &msg,
                                         CastKind &Kind) {
   bool IsLValueCast = false;
   
   DestType = Self.Context.getCanonicalType(DestType);
   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>()) {
     if (!SrcExpr.get()->isGLValue()) {
       // Cannot cast non-glvalue to (lvalue or rvalue) reference type. See the
       // similar comment in const_cast.
       msg = diag::err_bad_cxx_cast_rvalue;
       return TC_NotApplicable;

cfe/trunk/lib/Sema/SemaOverload.cpp

if (FunctionDecl *FunDecl = dyn_cast<FunctionDecl>(Fn)) {
         HasComplained |= Complain;
         return false;
       }
 
       if (!S.checkAddressOfFunctionIsAvailable(FunDecl))
         return false;
 
       QualType ResultTy;
-      if (Context.hasSameUnqualifiedType(TargetFunctionType, 
+      if (Context.hasSameUnqualifiedType(TargetFunctionType,
                                          FunDecl->getType()) ||
           S.IsNoReturnConversion(FunDecl->getType(), TargetFunctionType,
                                  ResultTy) ||
           (!S.getLangOpts().CPlusPlus && TargetType->isVoidPointerType())) {
         Matches.push_back(std::make_pair(
             CurAccessFunPair, cast<FunctionDecl>(FunDecl->getCanonicalDecl())));
         FoundNonTemplateFunction = true;
         return true;
Sema::ResolveAddressOfOverloadedFunction(Expr *AddressOfExpr,
   }
 
   if (pHadMultipleCandidates)
     *pHadMultipleCandidates = Resolver.hadMultipleCandidates();
   return Fn;
 }
 
 /// \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
 /// template, where that template-id refers to a single template whose template
 /// arguments are either provided by the template-id or have defaults,
 /// as described in C++0x [temp.arg.explicit]p3.
 ///
 /// If no template-ids are found, no diagnostics are emitted and NULL is

cfe/trunk/test/SemaCXX/enable_if.cpp

namespace FnPtrs {
   void test8() {
     int (*p)(int) = noOvlNoCandidate; // expected-error{{cannot take address of function 'noOvlNoCandidate' becuase it has one or more non-tautological enable_if conditions}}
     int (*p2)(int) = &noOvlNoCandidate; // expected-error{{cannot take address of function 'noOvlNoCandidate' becuase it has one or more non-tautological enable_if conditions}}
     int (*a)(int);
     a = noOvlNoCandidate; // expected-error{{cannot take address of function 'noOvlNoCandidate' becuase it has one or more non-tautological enable_if conditions}}
     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 ()'}}
+}
+}

cfe/trunk/test/SemaCXX/unaddressable-functions.cpp

+// 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}}
+}
+
+namespace unavailable {
+// Ensure that we check that the function can be called
+void foo() __attribute__((unavailable("don't call this")));
+void foo(int) __attribute__((enable_if(false, "")));
+
+void *Ptr = reinterpret_cast<void*>(foo); // expected-error{{'foo' is unavailable: don't call this}} expected-note@-3{{explicitly marked unavailable here}}
+}