[clang] [Clang] __has_builtin should return false for aux triple builtins (PR #121839)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 22 09:34:16 PST 2025


================
@@ -1818,8 +1819,21 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
             // usual allocation and deallocation functions. Required by libc++
             return 201802;
           default:
+            // We may get here because of aux builtins which may not be
+            // supported on the default target, for example if we have an X86
+            // specific builtin and the current target is SPIR-V. Sometimes we
+            // rely on __has_builtin returning true when passed a builtin that
+            // is not supported on the default target due to LangOpts but is
+            // supported on the aux target. See
+            // test/Headers/__cpuidex_conflict.c for an example. If the builtin
+            // is an aux builtin and it can never be supported on the default
+            // target, __has_builtin should return false.
+            if (getBuiltinInfo().isAuxBuiltinID(BuiltinID) &&
+                getBuiltinInfo().isAuxBuiltinIDAlwaysUnsupportedOnDefaultTarget(
----------------
AaronBallman wrote:

I think this behavior (https://godbolt.org/z/oEx4Earqj) is surprising in that it's unclear to me why `__cpuidex` is exposed as a builtin given that it's only a builtin on all MS languages (https://github.com/llvm/llvm-project/blob/f63e8ed16ef1fd2deb80cd88b5ca9d5b631b1c36/clang/include/clang/Basic/BuiltinsX86.td#L4494) and the aux triple that is given is not an MS language (it's x86_64-unknown-linux-gnu). It would make sense to me if the user passed `-fms-extensions` (or `-fms-compatibility` I suppose). The fact that `__has_builtin` returns `true` and we get the error *does* make sense (it would be weird for it to return false and still get the error or for it to return true and not get the error), but I think that `__has_builtin` should return `false` in the case where MS language extensions aren't enabled.

> The change in question uses the ifdef to try to prevent double definitions here,

Yup!

https://github.com/llvm/llvm-project/pull/121839


More information about the cfe-commits mailing list