[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: lib/Sema/SemaDecl.cpp:12449
   // function declaration is going to be treated as an error.
-  if (Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error) {
+  if (!getLangOpts().OpenCL &&
+      Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error) {
----------------
echuraev wrote:
> Anastasia wrote:
> > 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:
(a) postpose adding items into `Context.Idents` to `Sema`.
(b) extend `IdentifierTable` interface to support removing items (although it might not align well with the general concept).

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.


https://reviews.llvm.org/D31745