What happens if user declared a function without parameter as void f(); instead of void f(void) then try to use it? Will this be treated as implicit declaration and results in an error instead of warning?
But this is already in OpenCL group. Please, see line 8232. In this line is a comment that OpenCL warnings and errors are below.
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:
Ah yes! Sorry, I got confused by the absence of opencl in the neighboring diagnostics. :)
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.
I run this test and got a log with errors. The full log you can find in the end of the message.
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...
I disabled adding note diagnostic for OpenCL. Addition function to dictionary occurs in function CurrectTypo. In this function take place creation of consumer by calling function makeTypoCorrectionConsumer. The function do some checks and next create consumer by the following code:
auto Consumer = llvm::make_unique<TypoCorrectionConsumer>( *this, TypoName, LookupKind, S, SS, std::move(CCC), MemberContext, EnteringContext);
In constructor TypoCorrectionConsumer occurs adding correction to dictionary (ValidatedCorrections).
This way prevents the typo corrections completely for OpenCL which is not very desirable. I was just wondering could we prevent adding the invalid builtin function identifiers instead to the correction candidate list.
Like when work_group_reserve_read_pipe was first parsed it shouldn't have been added to the list of valid function identifiers to appear in the corrections of 'work_group_reserve_write_pipe'. I am guessing the identifier might be added when builtins are initialized...
Yes, sorry, I didn't think about it. I investigated the question how can I remove invalid functions from correction candidate list. But I have a problem and I hope that you will be able to help me.
I found that the correction is added in function void TypoCorrectionConsumer::addName (in file SemaLookup.cpp) which called from loop in function std::unique_ptr<TypoCorrectionConsumer> Sema::makeTypoCorrectionConsumer. The loop you can see below:
// For unqualified lookup, look through all of the names that we have // seen in this translation unit. // FIXME: Re-add the ability to skip very unlikely potential corrections. for (const auto &I : Context.Idents) Consumer->FoundName(I.getKey());
But the map Context.Idents already contains names of implicit functions. So, I found that names of functions were added to this dictionary during parsing AST. After calling ConsumeToken() function in void Parser::ParseDeclarationSpecifiers (in file ParseDecl.cpp) implicit functions were added to the dictionary. But in this function I'm not able to check is the OpenCL function implicit declared or not.
As a result I tried to remove names of implicit functions before calling CorrectTypo. But unfortunately, we don't have an API for removing items from Context.Idents. So, I don't know the good way for fixing these diagnostic messages. Could you help me please? Do you have any suggestions?
Thank you in advance!
Thanks for looking at this!
It seems like we are adding the identifiers to the map too early. So we could:
Although (b) feels more natural way conceptually, it might require bigger changes than (a).
Both need discussion in cfe-dev to continue, so it should be addressed in isolation. If you undo the last change, I am happy to approve the review.