[libcxx-commits] [PATCH] D66301: [libcxx] __thread_id should be available when an external threading API is used.

[Michael Platings via Phabricator via libcxx-commits]

michaelplatings added a comment.

In D66301#1636506 <https://reviews.llvm.org/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.


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

https://reviews.llvm.org/D66301