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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: lib/Lex/PPMacroExpansion.cpp:1923
+              Tok, *this, diag::err_feature_check_malformed);
+          return II ? getTargetInfo().getTriple().getArchName().equals_lower(
+                          II->getName())
----------------
compnerd wrote:
> 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?
I suppose `__is_target_arch(arm)` should probably work in that case too, especially seeing that we define the ARM macros already. The users would just have to differentiate between thumb and non-thumb by checking if `__is_target_arch(thumb)` succeeds too.


https://reviews.llvm.org/D41087