Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[hans]

Summary

MSVC's libc doesn't provide thread.h, so we should set the macro to indicate that.

We could just set it in C mode, but I noticed that Darwin sets it unconditionally, so perhaps we should do the same here.

[aaron.ballman]

Thanks for this! Should we similarly handle __STDC_NO_ATOMICS__, __STDC_NO_COMPLEX__, et al at the same time? Also, how should we handle __STDC_VERSION__? We set that in all C modes, but MSVC only sets it when passed /std:c11, etc explicitly.

[rnk]

lgtm

[aaron.ballman]

Also, one concern I have for this is what to do if/when the Microsoft CRT gets any of these features. Clang will be unconditionally setting __STDC_NO_WHATEVER__, which we can correct for newer versions of Clang, but still means older versions of Clang on those updated systems then report something nonsensical.

It'd be nice if Microsoft had a solution similar to stdc-predef.h used by glibc, so the CRT could define information that we could use to determine what macros to predefine rather than having to guess.

[hans]

In D112081#3073067, @aaron.ballman wrote:

Thanks for this! Should we similarly handle __STDC_NO_ATOMICS__, __STDC_NO_COMPLEX__, et al at the same time?

I'll look into those, thanks.

Also, how should we handle __STDC_VERSION__? We set that in all C modes, but MSVC only sets it when passed /std:c11, etc explicitly.

In any case we should probably do it separately. I don't really like MSVC's model where it's in a "traditional C with extensions" mode by default, but I clang-cl already also kind of does that because we don't define STDC when targeting Windows. But it looks like unlike MSVC we also don't define it when passing /Za. And, I wonder how /Za and /std: are supposed to interact.

In D112081#3073075, @aaron.ballman wrote:

Also, one concern I have for this is what to do if/when the Microsoft CRT gets any of these features. Clang will be unconditionally setting __STDC_NO_WHATEVER__, which we can correct for newer versions of Clang, but still means older versions of Clang on those updated systems then report something nonsensical.

It'd be nice if Microsoft had a solution similar to stdc-predef.h used by glibc, so the CRT could define information that we could use to determine what macros to predefine rather than having to guess.

I think the best we can do is make updates in future versions of Clang.

[rjmccall]

The standard answer is that compilers are designed to work with a specific set of system headers. In the Clang-on-Windows case, that's complicated by the fact that many people acquire Clang separately from the rest of the build environment (although Microsoft does distribute Clang officially now, right?), but I think the standard answer is still ultimately the correct one: Clang is designed to support the MSVC system headers that are currently out in the world, and whenever Microsoft releases new system headers, it's possible that you will need a new version of Clang.

As an aside, it's unfortunate that the name of this define makes it sound like not defining it implies that the environment is not concurrent. Oh well; if the standard says it's tied to the header, then obviously it's right to not define it.

[aaron.ballman]

In D112081#3073291, @rjmccall wrote:

The standard answer is that compilers are designed to work with a specific set of system headers. In the Clang-on-Windows case, that's complicated by the fact that many people acquire Clang separately from the rest of the build environment (although Microsoft does distribute Clang officially now, right?), but I think the standard answer is still ultimately the correct one: Clang is designed to support the MSVC system headers that are currently out in the world, and whenever Microsoft releases new system headers, it's possible that you will need a new version of Clang.

As an aside, it's unfortunate that the name of this define makes it sound like not defining it implies that the environment is not concurrent. Oh well; if the standard says it's tied to the header, then obviously it's right to not define it.

Yeah... I wish it was this clean and clear cut. :-D

N2731 (the latest C2x working draft) says in 6.10.8.3p1:

__STDC_NO_THREADS__ The integer constant 1, intended to indicate that the implementation does not support the <threads.h> header.

Which seems pretty straight-forward, until you get to 7.26.1p2:

Implementations that define the macro __STDC_NO_THREADS__ need not provide this header nor support any of its facilities.

and those facilities includes thread_local (the macro that expands to _Thread_local, the keyword), which got me wondering whether _Thread_local has any relationship with __STDC_NO_THREADS__. The standard did not claim there was, which supports the changes in this patch, but I remember this discussion happening in WG14. I dug around and found http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2291.htm that deals with exactly this question, and there was no consensus to adopt this proposal in WG14, so I eventually convinced myself this implementation is valid.

LGTM. :-)

[rnk]

@hans, I don't think you landed this. Should we do it? I came here after reviewing my issues on github: https://github.com/llvm/llvm-project/issues/48048

[hans]

Thanks for the reminder! Landed now.

[emmenlau]

Current OpenSSL 1.1.1m requires __STDC_NO_ATOMICS__ to build with clang-cl against MSVC. Is this on-topic here?

[rnk]

In D112081#3330585, @emmenlau wrote:

Current OpenSSL 1.1.1m requires __STDC_NO_ATOMICS__ to build with clang-cl against MSVC. Is this on-topic here?

I think stdatomic.h is supposed to work with clang-cl, see this comment here:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdatomic.h#L16
If it's not working, I'd recommend filing a github issue with more details about any compile failures.

[emmenlau]

I can confirm that the problem in OpenSSL is resolved as of OpenSSL 1.1.1s and ClangCl 15.0.6.

clang/lib/Basic/Targets/OSTargets.cpp

if (Opts.MicrosoftExt) {
     if (Opts.CPlusPlus11) {
       Builder.defineMacro("_RVALUE_REFERENCES_V2_SUPPORTED");
       Builder.defineMacro("_RVALUE_REFERENCES_SUPPORTED");
       Builder.defineMacro("_NATIVE_NULLPTR_SUPPORTED");
     }
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 
   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.
   // The value given is the Windows Code Page Identifier:
   // https://docs.microsoft.com/en-us/windows/win32/intl/code-page-identifiers
   //
   // Clang currently only supports UTF-8, so we'll use 65001
   Builder.defineMacro("_MSVC_EXECUTION_CHARACTER_SET", "65001");

clang/test/Preprocessor/init.c

 //
 // RUN: %clang_cc1 -fms-extensions -triple i686-pc-win32 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix MSEXT %s
 //
 // MSEXT-NOT:#define __STDC__
 // MSEXT:#define _INTEGRAL_MAX_BITS 64
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT:#define _MSVC_EXECUTION_CHARACTER_SET 65001
+// MSEXT:#define __STDC_NO_THREADS__ 1
 //
 //
 // RUN: %clang_cc1 -x c++ -fms-extensions -triple i686-pc-win32 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix MSEXT-CXX %s
 //
 // MSEXT-CXX:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-CXX:#define _WCHAR_T_DEFINED 1
 // MSEXT-CXX:#define __BOOL_DEFINED 1
 //