Skip to content

Conversation

@cjdb
Copy link
Contributor

@cjdb cjdb commented Jul 3, 2024

Clang Tidy flagged the following as problematic when running tests locally.

Clang Tidy flagged the following as problematic when running tests
locally.
@cjdb cjdb added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 3, 2024
@cjdb cjdb requested a review from a team as a code owner July 3, 2024 11:17
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2024

@llvm/pr-subscribers-libcxx

Author: Christopher Di Bella (cjdb)

Changes

Clang Tidy flagged the following as problematic when running tests locally.


Full diff: https://github.com/llvm/llvm-project/pull/97556.diff

3 Files Affected:

  • (modified) libcxx/include/__functional/function.h (+3-3)
  • (modified) libcxx/include/__thread/id.h (+8-8)
  • (modified) libcxx/include/__thread/support/pthread.h (+1-1)
diff --git a/libcxx/include/__functional/function.h b/libcxx/include/__functional/function.h
index c7b98035e34bf..cb6ad7352e716 100644
--- a/libcxx/include/__functional/function.h
+++ b/libcxx/include/__functional/function.h
@@ -754,7 +754,7 @@ class __func<_Rp1 (^)(_ArgTypes1...), _Alloc, _Rp(_ArgTypes...)> : public __base
 #    ifdef _LIBCPP_HAS_OBJC_ARC
       : __f_(__f)
 #    else
-      : __f_(reinterpret_cast<__block_type>(__f ? _Block_copy(__f) : nullptr))
+      : __f_(reinterpret_cast<__block_type>(__f ? std::__function::_Block_copy(__f) : nullptr))
 #    endif
   {
   }
@@ -765,7 +765,7 @@ class __func<_Rp1 (^)(_ArgTypes1...), _Alloc, _Rp(_ArgTypes...)> : public __base
 #    ifdef _LIBCPP_HAS_OBJC_ARC
       : __f_(__f)
 #    else
-      : __f_(reinterpret_cast<__block_type>(__f ? _Block_copy(__f) : nullptr))
+      : __f_(reinterpret_cast<__block_type>(__f ? std::__function::_Block_copy(__f) : nullptr))
 #    endif
   {
   }
@@ -786,7 +786,7 @@ class __func<_Rp1 (^)(_ArgTypes1...), _Alloc, _Rp(_ArgTypes...)> : public __base
   _LIBCPP_HIDE_FROM_ABI_VIRTUAL virtual void destroy() _NOEXCEPT {
 #    ifndef _LIBCPP_HAS_OBJC_ARC
     if (__f_)
-      _Block_release(__f_);
+      std::__function::_Block_release(__f_);
 #    endif
     __f_ = 0;
   }
diff --git a/libcxx/include/__thread/id.h b/libcxx/include/__thread/id.h
index 6db0ccbfe569b..6b014720ce05e 100644
--- a/libcxx/include/__thread/id.h
+++ b/libcxx/include/__thread/id.h
@@ -42,17 +42,17 @@ class _LIBCPP_TEMPLATE_VIS __thread_id {
 
   static _LIBCPP_HIDE_FROM_ABI bool
   __lt_impl(__thread_id __x, __thread_id __y) _NOEXCEPT { // id==0 is always less than any other thread_id
-    if (__x.__id_ == 0)
-      return __y.__id_ != 0;
-    if (__y.__id_ == 0)
+    if (__x.__id_ == nullptr)
+      return __y.__id_ != nullptr;
+    if (__y.__id_ == nullptr)
       return false;
     return __libcpp_thread_id_less(__x.__id_, __y.__id_);
   }
 
 public:
-  _LIBCPP_HIDE_FROM_ABI __thread_id() _NOEXCEPT : __id_(0) {}
+  _LIBCPP_HIDE_FROM_ABI __thread_id() _NOEXCEPT : __id_(nullptr) {}
 
-  _LIBCPP_HIDE_FROM_ABI void __reset() { __id_ = 0; }
+  _LIBCPP_HIDE_FROM_ABI void __reset() { __id_ = nullptr; }
 
   friend _LIBCPP_HIDE_FROM_ABI bool operator==(__thread_id __x, __thread_id __y) _NOEXCEPT;
 #  if _LIBCPP_STD_VER <= 17
@@ -77,9 +77,9 @@ class _LIBCPP_TEMPLATE_VIS __thread_id {
 
 inline _LIBCPP_HIDE_FROM_ABI bool operator==(__thread_id __x, __thread_id __y) _NOEXCEPT {
   // Don't pass id==0 to underlying routines
-  if (__x.__id_ == 0)
-    return __y.__id_ == 0;
-  if (__y.__id_ == 0)
+  if (__x.__id_ == nullptr)
+    return __y.__id_ == nullptr;
+  if (__y.__id_ == nullptr)
     return false;
   return __libcpp_thread_id_equal(__x.__id_, __y.__id_);
 }
diff --git a/libcxx/include/__thread/support/pthread.h b/libcxx/include/__thread/support/pthread.h
index 531f3e71de839..94ba471027ed0 100644
--- a/libcxx/include/__thread/support/pthread.h
+++ b/libcxx/include/__thread/support/pthread.h
@@ -175,7 +175,7 @@ inline _LIBCPP_HIDE_FROM_ABI __libcpp_thread_id __libcpp_thread_get_id(const __l
 }
 
 inline _LIBCPP_HIDE_FROM_ABI bool __libcpp_thread_isnull(const __libcpp_thread_t* __t) {
-  return __libcpp_thread_get_id(__t) == 0;
+  return __libcpp_thread_get_id(__t) == nullptr;
 }
 
 inline _LIBCPP_HIDE_FROM_ABI int __libcpp_thread_create(__libcpp_thread_t* __t, void* (*__func)(void*), void* __arg) {

@cjdb
Copy link
Contributor Author

cjdb commented Jul 3, 2024

This only seems to be an issue on my Mac. I get build errors on Linux?

: __f_(__f)
# else
: __f_(reinterpret_cast<__block_type>(__f ? _Block_copy(__f) : nullptr))
: __f_(reinterpret_cast<__block_type>(__f ? std::__function::_Block_copy(__f) : nullptr))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: __f_(reinterpret_cast<__block_type>(__f ? std::__function::_Block_copy(__f) : nullptr))
: __f_(reinterpret_cast<__block_type>(__f ? __function::_Block_copy(__f) : nullptr))

if (__x.__id_ == 0)
return __y.__id_ != 0;
if (__y.__id_ == 0)
if (__x.__id_ == nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like __x.__id_ isn't a pointer on other platforms. I guess we just have to disable the check for this file.

@ldionne
Copy link
Member

ldionne commented Aug 19, 2024

Gentle ping on this!

@ldionne
Copy link
Member

ldionne commented Nov 28, 2024

Closing as stale, please reopen if you'd like to pursue!

@ldionne ldionne closed this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants