[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Mario Emmenlauer via Phabricator via cfe-commits]

emmenlau added a comment.
Herald added a project: All.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Mario Emmenlauer via Phabricator via cfe-commits]

emmenlau added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks for the reminder! Landed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbc690c57213: Define __STDC_NO_THREADS__ when targeting 
windows-msvc (PR48704) (authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D112081?vs=380704=394877#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -195,6 +195,7 @@
 // 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
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -203,6 +203,7 @@
   }
 
   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.


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -195,6 +195,7 @@
 // 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
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -203,6 +203,7 @@
   }
 
   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.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

@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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

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  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. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rsmith, jyknight, rjmccall.
aaron.ballman added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: aaron.ballman, rnk.
hans requested review of this revision.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112081

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -192,6 +192,7 @@
 //
 // MSEXT-NOT:#define __STDC__
 // MSEXT:#define _INTEGRAL_MAX_BITS 64
+// MSEXT:#define __STDC_NO_THREADS__ 1
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 //
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -201,6 +201,7 @@
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 }
 
 void addWindowsDefines(const llvm::Triple , const LangOptions ,


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -192,6 +192,7 @@
 //
 // MSEXT-NOT:#define __STDC__
 // MSEXT:#define _INTEGRAL_MAX_BITS 64
+// MSEXT:#define __STDC_NO_THREADS__ 1
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 //
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -201,6 +201,7 @@
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 }
 
 void addWindowsDefines(const llvm::Triple , const LangOptions ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits