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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
