[OpenCL] Allow function declaration with empty argument list.

[bader]

Summary

Treat 'f()' as 'f(void)' rather than a function w/o a prototype.

[yaxunl]

I kind of like this change. In reality, people often use f() as an alternative of f(void). I think diagnose it as an error is just causing inconvenience.

[Anastasia]

In D33681#767957, @yaxunl wrote:

I kind of like this change. In reality, people often use f() as an alternative of f(void). I think diagnose it as an error is just causing inconvenience.

Yes, I also think it's useful in general. Just wondering whether we should propagate it to Khronos too since it's a deviation from either C99 or OpenCL spec.

[bader]

Sorry for the delay.
It will also align the behavior with function definition.
For instance the following function definition is allowed.

__local int *allocateLocalMemory() { return 0; }

Regarding propagating this to Khronos, I'm not sure if this feature requires additional clarification in the specification.
My understanding is that it's clear that 'foo()' is equivalent to 'foo(void)'.
Do you have any particular proposal to the specification text in mind?

[Anastasia]

In D33681#771161, @bader wrote:

Sorry for the delay.
It will also align the behavior with function definition.
For instance the following function definition is allowed.

__local int *allocateLocalMemory() { return 0; }

Regarding propagating this to Khronos, I'm not sure if this feature requires additional clarification in the specification.
My understanding is that it's clear that 'foo()' is equivalent to 'foo(void)'.
Do you have any particular proposal to the specification text in mind?

We could steal some text from C++? Or we can create a ticket to Khronos and see if they resolve this. Otherwise, I am also ok to keep this as Clang "extension".

Should we add cfe-commits then?

[Anastasia]

LGTM! Thanks!

[bader]

Update one more test missed in the first version of the patch.

Actually it looks like a bug in Parser.
Clang interprets following statement as variable declaration.

extern pipe write_only int get_pipe();

So the type of the pipe element is function prototype: int get_pipe().

If I understand the intention correctly, clang is expected to treat this expression function declaration with pipe return type, which elements are int.

Would it acceptable to commit this patch as is and file another bug on Parser?
I need more time to think how to resolve this ambiguity.

[bader]

Ping.

Although this patch is already accepted, I'd like to confirm that I can commit the latest update with changes in test/SemaOpenCL/invalid-pipes-cl2.0.cl.

Thanks.

[Anastasia]

Sure, no problem! We might add some note on this feature to Clang manual. But we can do it later as well. Thanks!

[chapuni]

Reverted in rL306660.
See also; http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/4926

[bader]

@chapuni, thanks for taking care of this. I'll take a look.

[bader]

Fix parsing of function declarations with empty parameter list initializers.
This change should fix the failure caught by the buildbot.
Sorry for the long delay.

[Anastasia]

I would prefer to merge the new test with test/SemaOpenCL/func.cl, but otherwise LGTM! Thanks!

[yaxunl]

LGTM. Thanks.

[bader]

Applied comments.

lib/Parse/ParseDecl.cpp

if (isFunctionDeclaratorIdentifierList()) {
     EndLoc = RParenLoc;
   } else {
     if (Tok.isNot(tok::r_paren))
       ParseParameterDeclarationClause(D, FirstArgAttrs, ParamInfo, 
                                       EllipsisLoc);
     else if (RequiresArg)
       Diag(Tok, diag::err_argument_required_after_attribute);
 
-    HasProto = ParamInfo.size() || getLangOpts().CPlusPlus;
+    HasProto = ParamInfo.size() || getLangOpts().CPlusPlus
+                                || getLangOpts().OpenCL;
 
     // If we have the closing ')', eat it.
     Tracker.consumeClose();
     RParenLoc = Tracker.getCloseLocation();
     LocalEndLoc = RParenLoc;
     EndLoc = RParenLoc;
 
     if (getLangOpts().CPlusPlus) {

lib/Sema/SemaType.cpp

case DeclaratorChunk::Function: {
 
       // If we see "T var();" or "T var(T());" at block scope, it is probably
       // an attempt to initialize a variable, not a function declaration.
       if (FTI.isAmbiguous)
         warnAboutAmbiguousFunction(S, D, DeclType, T);
 
       FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
-      if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
+      if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus
+                                            && !LangOpts.OpenCL) {
         // Simple void foo(), where the incoming T is the result type.
         T = Context.getFunctionNoProtoType(T, EI);
       } else {
         // We allow a zero-parameter variadic function in C if the
         // function is marked with the "overloadable" attribute. Scan
         // for this attribute now.
         if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus) {
           bool Overloadable = false;

test/SemaOpenCL/func.cl

-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -triple spir-unknown-unknown
 
 // Variadic functions
 void vararg_f(int, ...);                    // expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
 void __vararg_f(int, ...);
 typedef void (*vararg_fptr_t)(int, ...);    // expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
                                             // expected-error@-1{{pointers to functions are not allowed}}
 int printf(__constant const char *st, ...); // expected-error {{invalid prototype, variadic arguments are not allowed in OpenCL}}
 
 // Struct type with function pointer field
 typedef struct s
 {
    void (*f)(struct s *self, int *i);       // expected-error{{pointers to functions are not allowed}}
 } s_t;
 
 //Function pointer
 void foo(void*);
 
+// Expect no diagnostics for an empty parameter list.
+void bar();
+
 void bar()
 {
   // declaring a function pointer is an error
   void (*fptr)(int); // expected-error{{pointers to functions are not allowed}}
 
   // taking the address of a function is an error
   foo((void*)foo); // expected-error{{taking address of function is not allowed}}
   foo(&foo); // expected-error{{taking address of function is not allowed}}
 
   // initializing an array with the address of functions is an error
   void* vptrarr[2] = {foo, &foo}; // expected-error{{taking address of function is not allowed}} expected-error{{taking address of function is not allowed}}
 
   // just calling a function is correct
   foo(0);
 }

test/SemaOpenCL/invalid-pipes-cl2.0.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
 global pipe int gp;            // expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;          // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
 
 void test1(pipe int *p) {// expected-error {{pipes packet types cannot be of reference type}}
 }
 void test2(pipe p) {// expected-error {{missing actual type specifier for pipe}}
 }