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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

It would be good to straighten out the corner case of the canonicalized vs specified triple before merging this as that would make it harder to change.

Minor nit with the style, I'm not too fond of the longer lambdas being inlined, having a separate function being passed instead would be easier to read I think.



================
Comment at: lib/Lex/PPMacroExpansion.cpp:1923
+              Tok, *this, diag::err_feature_check_malformed);
+          return II ? getTargetInfo().getTriple().getArchName().equals_lower(
+                          II->getName())
----------------
arphaman wrote:
> compnerd wrote:
> > Hmm, the one thing to consider here is the canonicalized vs spelt target.  e.g. `armv7-windows` will map to `thumbv7-unknown-windows-msvc`.
> I think it's ok to only allow "thumb" check to succeed instead of "arm", otherwise how would we differentiate between the two? However, we should take the sub arch into account, so when arch is "thumbv7", these checks should succeed:
> 
> ```
> __is_target_arch(thumb)
> __is_target_arch(thumbv7)
> ```
> 
> but this one should fail:
> 
> ```
> __is_target_arch(thumbv6)
> ```
> 
> I fixed this in the updated patch.
I'm considering the scenario where the user specifies `-target armv7-windows` and the `__is_target_arch(thumb)` passes but `__is_target_arch(arm)` fails.  Is that really what people would expect?


================
Comment at: test/Preprocessor/is_target_os_darwin.c:22
+
+#if __is_target_os(ios)
+  #error "mismatching os"
----------------
Is this supposed to be within the `MAC` clause?


https://reviews.llvm.org/D41087