[PATCH] D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added a comment.

Yes, in general this approach looks good to me conceptually. I have two suggestions:

1. As we discussed, the term //core functionality// should be revisited here. There's no clear meaning about that in the spec and I think interpreting it as //supported by default// is a little dangerous. So //core// (AFAIK) means that it was just promoted to a core specification thus is still remains optional by targets.
2. Sort of a implementation suggestion. I understand that double-scored identifiers are reserved for any use, but still, can defining such macro as `__undef_cl_khr_depth_images ` be avoided? We could use `Preproceccor` class for the things that you are proposing to do. I was trying to do something similar when implementing features and I tried something like (`Preprocessor::appendDefMacroDirective` already exists):



  UndefMacroDirective *Preprocessor::appendUndefMacroDirective(IdentifierInfo *II,
                                 SourceLocation Loc) {
     if (!II->hasMacroDefinition())
       return nullptr;
     UndefMacroDirective *UD = AllocateUndefMacroDirective(Loc);
     appendMacroDirective(II, UD);
     return UD;
  }

I tried to handle some special pragma in this way and it worked. So maybe this can be reused without binding to any specific `SourceLocation`? But maybe there is an other strong concern why `Preprocessor::appendUndefMacroDirective` still doesn't exist...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91531/new/

https://reviews.llvm.org/D91531