[PATCH] D125555: [clang] Add __has_target_feature

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:260
 
+``__has_target_feature``
+------------------------
----------------
yaxunl wrote:
> aaron.ballman wrote:
> > The first question that comes to mind for me is: why is `__has_feature` not sufficient?
> `__has_target_feature` accepts predefined target features for the specified target triple and CPU. The names of target features are determined by the target. They may overlap with clang features and cause ambiguity. Another issue is that they usually contain '-', which makes them not valid identifiers of C/C++, therefore the builtin macro has to accept a string literal argument instead of identifier argument like `__has_feature`. 
Do we have to use a string literal? I am presuming there's an enumerated list of features for each target, and thus we can use an identifier argument (it might turn around and map that identifier back to a string literal, but that's fine). But I'm not certain I understand how a target feature will overlap with a clang feature and cause ambiguity.

My concerns really boil down to:

* It's already hard enough for users to know when to use `__has_extension` vs `__has_feature` and this is adding another option into the mix.
* There's no documentation about what constitutes a valid string literal for this feature and I don't know how a user would go about discovering the full enumerated list for any given target. Also, are these strings guaranteed to be stable (spelling doesn't change, strings aren't removed, etc)?
* (much smaller concern) A string literal may not be the most user-friendly way to surface the functionality (tools typically don't auto-complete string literals or do typo correction within them).



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

https://reviews.llvm.org/D125555