[libcxx] __thread_id should be available when an external threading API is used.

[DavidSpickett]

Summary

Commit r368867 "Fix a layering violation in mutex"
moved "thread_id" from <thread> to <threading_support>.

Along the way the conditions for its presence changed.
Before it only required that threading was supported.
Now it needs that and you not to be using an external
API.

This change restores the old behaviour by putting it
in a separate block with the correct condition.

[zoecarver]

libcxx/include/__threading_support

398

Any reason this cannot go at the top of the file before _LIBCPP_BEGIN_NAMESPACE_STD?

[mclow.lists]

Yes. Because then it would be in the global namespace.

[zoecarver]

In D66301#1635254, @mclow.lists wrote:

Yes. Because then it would be in the global namespace.

(assuming you were responding to me) Isn't that the intent? It looks like it's just conditionally adding an #include.

[michaelplatings]

In D66301#1636506, @zoecarver wrote:

Isn't that the intent? It looks like it's just conditionally adding an #include.

No, that's not the intent. There may be some misunderstanding going on here so I'll have a go at explaining the intent in my own words (I worked with David on this patch).

Near the top of the file there's:

#if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
# include <__external_threading>
#elif !defined(_LIBCPP_HAS_NO_THREADS)

this was followed by the rest of the code in the header. Since __thread_id et al. was moved into this header from another header (about a week ago), it's only compiled if neither _LIBCPP_HAS_THREAD_API_EXTERNAL nor _LIBCPP_HAS_NO_THREADS are defined whereas previously it had been compiled irrespective of _LIBCPP_HAS_THREAD_API_EXTERNAL.
To restore the previous behaviour, we're ending the #if block (#endif // !_LIBCPP_HAS_NO_THREADS and !_LIBCPP_HAS_THREAD_API_EXTERNAL) and starting a new one (#if !defined(_LIBCPP_HAS_NO_THREADS)), which removes the _LIBCPP_HAS_THREAD_API_EXTERNAL condition.

The _LIBCPP_END_NAMESPACE_STD stuff we're adding is only there to make sure there isn't a mismatch in the number of times we open or close the namespace, or push or pop macros.

Hope that makes things clearer.

[zoecarver]

Thank you for explaining. That makes a lot of sense.

[mclow.lists]

I put an alternate solution up as D66480

[DavidSpickett]

Abandoned in favour of Marshall's fix.

libcxx/include/__threading_support

 
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)
 {
     return pthread_setspecific(__key, __p);
 }
 
 #endif // !_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL || _LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL
 
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // !_LIBCPP_HAS_NO_THREADS and !_LIBCPP_HAS_THREAD_API_EXTERNAL
+
+#if !defined(_LIBCPP_HAS_NO_THREADS)
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
 class _LIBCPP_TYPE_VIS thread;
 class _LIBCPP_TYPE_VIS __thread_id;
 
 namespace this_thread
 {
 
 _LIBCPP_INLINE_VISIBILITY __thread_id get_id() _NOEXCEPT;