[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D41087#956379, @bob.wilson wrote:

> I'm concerned here about the check for the names as written vs. the canonical names. @compnerd pointed out one specific case with armv7, and I see that you've got special handling for "darwin", but I think there are more. What about "arm64" vs. the canonical "aarch64"? Look through the triple parsing code in Triple.cpp and I'm pretty sure you'll find more. I had been thinking about using the canonical names. However, that's not ideal either because the canonical names intentionally exclude suffixes that some users may want to distinguish (e.g., armv7 vs armv7k).


Ah, you're right. I should just check the arch instead of the arch name and the subarch too (for the "arm64" vs "aarch64"). I fixed that in r320853. I believe that other similar arch cases are already handled when there's no subarch.
I think that the other component that's troubling is environment because of the version number. I fixed that in r320854.

The "armv7" and "armv7k" will have to stay separate I think. If "armv7k" is the target arch then the user would either have to check for `__is_target_arch(arm)` or `__is_target_arch(armv7k)`, and `__is_target_arch(armv7)` will fail. That seems reasonable to me though.


Repository:
  rC Clang

https://reviews.llvm.org/D41087