[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: test/SemaOpenCL/clang-builtin-version.cl:32
+  work_group_reserve_write_pipe(tmp, tmp); // expected-error{{implicit declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL}}
+  // expected-note at -1{{did you mean 'work_group_reserve_read_pipe'?}}
+  // expected-note at -2{{'work_group_reserve_write_pipe' declared here}}
----------------
echuraev wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > echuraev wrote:
> > > > Anastasia wrote:
> > > > > Why do we get this note now? I believe work_group_reserve_read_pipe shouldn't be available either?
> > > > May be I don't understand something but I think that it is the right diagnostic message. We called work_group_reserve_read_pipe in line 29. So for this case we will get the following message: 
> > > > //clang-builtin-version.cl: 31 error: implicit declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL
> > > > clang-builtin-version.cl: 31 note: did you mean 'work_group_reserve_read_pipe'?
> > > > clang-builtin-version.cl: 29 note: 'work_group_reserve_read_pipe' declared here//
> > > But there is an error now given for the call to 'work_group_reserve_read_pipe'. Why is it still added to the declarations? I think we should prevent this.
> > Also do you know why we didn't have these notes before? I don't see anything related in your change.
> I run this test and got a log with errors. The full log you can find in the end of the message.
> I think that the diagnostic messages are right. First error we get for implicit declaration of **//work_group_reserve_read_pipe//**. After that we call function **//work_group_reserve_write_pipe//** and get the same error about implicit declaration of function. But also we get two diagnostic messages. Compiler already know about **//work_group_reserve_read_pipe//** and make a hypothesis: "//note: did you mean 'work_group_reserve_read_pipe'?//" and "//note: 'work_group_reserve_read_pipe' declared here//".
> As well, next we have declaration of **//work_group_commit_read_pipe//**. And we get an error about implicit declaration of this function and also get a note messages: "//note: did you mean 'work_group_reserve_read_pipe'?//" and "//note: 'work_group_reserve_read_pipe' declared here//".
> 
> 
> > Also do you know why we didn't have these notes before? I don't see anything related in your change.
> 
> Unfortunately, I don't know why we get these notes. They have appeared when I changed the status of diagnostic message for implicit declaration from warning to error. If we had only warning messages then we wouldn't have these notes.
> 
> 
> 
> 
> ```
> clang-builtin-version.cl:35:3: error: implicit declaration of function 'work_group_reserve_read_pipe' is invalid in OpenCL
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> 
> clang-builtin-version.cl:37:3: error: implicit declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL
>   work_group_reserve_write_pipe(tmp, tmp);
>   ^
> clang-builtin-version.cl:37:3: note: did you mean 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl:35:3: note: 'work_group_reserve_read_pipe' declared here
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> 
> clang-builtin-version.cl:50:3: error: implicit declaration of function 'work_group_commit_read_pipe' is invalid in OpenCL
>   work_group_commit_read_pipe(tmp, tmp);
>   ^
> clang-builtin-version.cl:50:3: note: did you mean 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl:35:3: note: 'work_group_reserve_read_pipe' declared here
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> ```
I find those notes counterintuitive to be honest because if there is an error given for the function declaration - let's say `foo()`, it shouldn't have been added anywhere to any dictionary because if the next function let's say `foa()` is corrected to match the name from the note (i.e. `foo()`) it is already known that it won't be compiled too. So to me this note seems wrong because it attempts to correct to something which is already known not to be compiled. I think it's worth digging into it a bit more...


https://reviews.llvm.org/D31745