[OpenCL] Add diagnostics for references to functions

[Anastasia]

Summary

References to functions are less permissive than pointers to functions and therefore some use cases could be allowed for example:

• For local variables
• For non-extern static and global variables
• For template parameters that are not used as function parameters or class members
• In constexpr

However, more language work is needed to analyze the invalid/valid cases. This can be done in the future language versions if there is enough interest in the feature from the application developers. Even if we had a request for allowing some function pointer functionality (PR44788), OpenCL has never adopted the feature as a part of the standard and therefore many applications have been written successfully without it. On the other hand C++ provides other related features - lambdas and function objects that are allowed in OpenCL and can be used to express some sort of the indirect function call logic. For now it seems reasonable to just disallow the references to functions just like we disallow function pointers. This prevents erroneous programs from being compiled silently.

Note that for the advanced users there is the following extension available __cl_clang_function_pointers that can be used if there is knowledge about the application sources or compilation options to make sure the non-conformant functionality is safe (more details are in https://clang.llvm.org/docs/LanguageExtensions.html#opencl-features).

[Anastasia]

clang/test/SemaOpenCLCXX/references.cl

25

I have created PR48887 for this because we also miss other diagnotics.

30

oops, I thought this was covered in my patch. I will see if there is a quick fix and if not I will create another PR.

[Anastasia]

Improved diagnostics to cover more cases.

NOTE that this now also contains similar improvements for the pointers to member functions.

[Anastasia]

clang/test/SemaOpenCLCXX/references.cl

30

Ok, I have addressed that. I realized that my original patch was not doing what I said in the description as it was still allowing the references to functions in type aliases or template arguments. While I think such functionality might be useful we should first start from the language definition side of it. For now let's just go with the conservative approach and disallow this.

[mantognini]

Looks sensible to me.

clang/test/SemaOpenCLCXX/members.cl

17

I wonder, do we lose coverage by removing these templates?

In other words, is the same code for error detection used for templates and non-template?

clang/test/SemaOpenCLCXX/references.cl

26

This re-uses the same name as above. Could that be an issue?

[Anastasia]

clang/test/SemaOpenCLCXX/members.cl

17

I just think that this pattern has become untestable which means it is also not customer visible i.e. I used these to create a function pointer type in a template via template argument that was a reference to function. But now that we don't have neither pointers nor references to functions I don't think it is possible to write a template that would have any of those two after the substitution? I think the error will occur before - either when we specify the template argument or create a variable of those invalid types to be deduced in the templates.

clang/test/SemaOpenCLCXX/references.cl

26

Not in this case because the type alias has not been created i.e. the parser just restarts over as if the first line with this type alias has not existed.

[mantognini]

Right, thanks for the explanations. They make sense.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_invalid_conversion_between_vectors : Error<
   "invalid conversion between vector type%diff{ $ and $|}0,1 of different "
   "size">;
 def err_invalid_conversion_between_vector_and_integer : Error<
   "invalid conversion between vector type %0 and integer type %1 "
   "of different size">;
 
 def err_opencl_function_pointer : Error<
-  "pointers to functions are not allowed">;
+  "%select{pointers|references}0 to functions are not allowed">;
 
 def err_opencl_taking_address_capture : Error<
   "taking address of a capture is not allowed">;
 
 def err_invalid_conversion_between_vector_and_scalar : Error<
   "invalid conversion between vector type %0 and scalar type %1">;
 
 // C++ member initializers.

clang/lib/Sema/SemaDecl.cpp

if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
           << R;
       D.setInvalidType();
       return false;
     }
   }
 
   // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
   if (!Se.getOpenCLOptions().isEnabled("__cl_clang_function_pointers")) {
-    QualType NR = R;
-    while (NR->isPointerType() || NR->isMemberFunctionPointerType()) {
-      if (NR->isFunctionPointerType() || NR->isMemberFunctionPointerType()) {
-        Se.Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer);
+    QualType NR = R.getCanonicalType();
+    while (NR->isPointerType() || NR->isMemberFunctionPointerType() ||
+           NR->isReferenceType()) {
+      if (NR->isFunctionPointerType() || NR->isMemberFunctionPointerType() ||
+          NR->isFunctionReferenceType()) {
+        Se.Diag(D.getIdentifierLoc(), diag::err_opencl_function_pointer)
+            << NR->isReferenceType();
         D.setInvalidType();
         return false;
       }
       NR = NR->getPointeeType();
     }
   }
 
   if (!Se.getOpenCLOptions().isEnabled("cl_khr_fp16")) {

clang/lib/Sema/SemaType.cpp

if (T->isReferenceType()) {
     // C++ 8.3.2p4: There shall be no ... pointers to references ...
     Diag(Loc, diag::err_illegal_decl_pointer_to_reference)
       << getPrintableNameForEntity(Entity) << T;
     return QualType();
   }
 
   if (T->isFunctionType() && getLangOpts().OpenCL &&
       !getOpenCLOptions().isEnabled("__cl_clang_function_pointers")) {
-    Diag(Loc, diag::err_opencl_function_pointer);
+    Diag(Loc, diag::err_opencl_function_pointer) << /*pointer*/ 0;
     return QualType();
   }
 
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
     return QualType();
 
   assert(!T->isObjCObjectType() && "Should build ObjCObjectPointerType");
 
QualType Sema::BuildReferenceType(QualType T, bool SpelledAsLValue,
   if (T->isVoidType()) {
     Diag(Loc, diag::err_reference_to_void);
     return QualType();
   }
 
   if (checkQualifiedFunction(*this, T, Loc, QFK_Reference))
     return QualType();
 
+  if (T->isFunctionType() && getLangOpts().OpenCL &&
+      !getOpenCLOptions().isEnabled("__cl_clang_function_pointers")) {
+    Diag(Loc, diag::err_opencl_function_pointer) << /*reference*/ 1;
+    return QualType();
+  }
+
   // In ARC, it is forbidden to build references to unqualified pointers.
   if (getLangOpts().ObjCAutoRefCount)
     T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ true);
 
   if (getLangOpts().OpenCL)
     T = deduceOpenCLPointeeAddrSpace(*this, T);
 
   // Handle restrict on references.
if (T->isVoidType()) {
     return QualType();
   }
 
   if (!Class->isDependentType() && !Class->isRecordType()) {
     Diag(Loc, diag::err_mempointer_in_nonclass_type) << Class;
     return QualType();
   }
 
+  if (T->isFunctionType() && getLangOpts().OpenCL &&
+      !getOpenCLOptions().isEnabled("__cl_clang_function_pointers")) {
+    Diag(Loc, diag::err_opencl_function_pointer) << /*pointer*/ 0;
+    return QualType();
+  }
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
       (Entity.getNameKind() == DeclarationName::CXXConstructorName) ||
       (Entity.getNameKind() == DeclarationName::CXXDestructorName);
   if (T->isFunctionType())
     adjustMemberFunctionCC(T, /*IsStatic=*/false, IsCtorOrDtor, Loc);
 

clang/test/SemaOpenCLCXX/members.cl

 //RUN: %clang_cc1 %s -triple spir -cl-std=clc++ -verify -fsyntax-only
 //RUN: %clang_cc1 %s -triple spir -cl-std=clc++ -verify -fsyntax-only -DFUNCPTREXT
 
 #ifdef FUNCPTREXT
 #pragma OPENCL EXTENSION __cl_clang_function_pointers : enable
 //expected-no-diagnostics
 #endif
 
 // Check that pointer to member functions are diagnosed
 // unless specific clang extension is enabled.
 struct C {
   void f(int n);
 };
 
 typedef void (C::*p_t)(int);
-
-template <class T> struct remove_reference { typedef T type; };
-template <class T> struct remove_reference<T &> { typedef T type; };
-
-template <typename T>
-void templ_test() {
-  typename remove_reference<T>::type *ptr;
 #ifndef FUNCPTREXT
-  //expected-error@-2{{pointers to functions are not allowed}}
 #endif
-}
 
 void test() {
   void (C::*p)(int);
 #ifndef FUNCPTREXT
 //expected-error@-2{{pointers to functions are not allowed}}
 #endif
-
-  p_t p1;
-#ifndef FUNCPTREXT
-//expected-error@-2{{pointers to functions are not allowed}}
-#endif
-
-  templ_test<int (&)()>();
-#ifndef FUNCPTREXT
-//expected-note@-2{{in instantiation of function template specialization}}
-#endif
 }

clang/test/SemaOpenCLCXX/references.cl

+//RUN: %clang_cc1 %s -cl-std=clc++ -verify -fsyntax-only
+//RUN: %clang_cc1 %s -cl-std=clc++ -verify -fsyntax-only -DFPTREXT
+
+#ifdef FPTREXT
+#pragma OPENCL EXTENSION __cl_clang_function_pointers : enable
+#endif // FPTREXT
+
+// References to functions are not allowed.
+struct myclass {
+//FIXME: Here we provide incorrect diagnostic.
+  void (&mem)(); //expected-error{{reference to function type cannot have '__generic' qualifier}}
+};
+
+void (&glob)();
+#ifndef FPTREXT
+//expected-error@-2{{references to functions are not allowed}}
+#else
+//expected-error@-4{{declaration of reference variable 'glob' requires an initializer}}
+#endif // FPTREXT
+
+using ref2fct_t = void (&)();
+#ifndef FPTREXT
+//expected-error@-2{{references to functions are not allowed}}
+#endif // FPTREXT
+typedef void (&ref2fct_t)();
+#ifndef FPTREXT
+//expected-error@-2{{references to functions are not allowed}}
+#endif // FPTREXT
+
+void test(void (&par)()) {
+#ifndef FPTREXT
+//expected-error@-2{{references to functions are not allowed}}
+#endif // FPTREXT
+  void (&loc)();
+#ifndef FPTREXT
+//expected-error@-2{{references to functions are not allowed}}
+#else
+//expected-error@-4{{declaration of reference variable 'loc' requires an initializer}}
+#endif // FPTREXT
+
+  void (*&ref2fptr)();
+#ifndef FPTREXT
+//expected-error@-2{{pointers to functions are not allowed}}
+#endif // FPTREXT
+//expected-error@-4{{declaration of reference variable 'ref2fptr' requires an initializer}}
+}