[PATCH] D125829: [clang] Fix __has_builtin

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 17 14:04:08 PDT 2022


tra added a subscriber: echristo.
tra added a comment.

@echristo - FYI, in case you have an opinion on target builtin feature checks.



================
Comment at: clang/include/clang/Basic/Builtins.h:263
+  ///    false if it is disabled.
+  bool isRequiredTargetFeaturesEnabled(
+      unsigned BuiltinID, const llvm::StringMap<bool> &TargetFetureMap) const;
----------------
I think we should boil it down to just `evaluateRequiredTargetFeatures(StringRef RequiredFeatures, ... TargetFeatureMap)`.



================
Comment at: clang/lib/Basic/BuiltinTargetFeatures.h:32
+/// pairs.
+class TargetFeatures {
+  struct FeatureListStatus {
----------------
I don't think it needs to be part of the public API. Feature check function operates on `llvm::StringMap<bool>` and does not need to know about this class, and the implementation of this class can be moved into `Builtins.cpp` where it's actually used.


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

https://reviews.llvm.org/D125829



More information about the cfe-commits mailing list