Skip to content

Conversation

@mordante
Copy link
Member

For certain time_points there are specializations of __convert_to_tm. This means the non-specialized version is never called. This means some of the if constexpr will never be true. These are removed.

Since there is a static_assert accidental removal of the specialization will make the code ill-formed.

@mordante mordante requested a review from a team as a code owner March 19, 2025 21:10
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 19, 2025

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

For certain time_points there are specializations of __convert_to_tm. This means the non-specialized version is never called. This means some of the if constexpr will never be true. These are removed.

Since there is a static_assert accidental removal of the specialization will make the code ill-formed.


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

1 Files Affected:

  • (modified) libcxx/include/__chrono/convert_to_tm.h (+4-11)
diff --git a/libcxx/include/__chrono/convert_to_tm.h b/libcxx/include/__chrono/convert_to_tm.h
index 0a6b627726091..23cb9b6518c86 100644
--- a/libcxx/include/__chrono/convert_to_tm.h
+++ b/libcxx/include/__chrono/convert_to_tm.h
@@ -143,21 +143,14 @@ _LIBCPP_HIDE_FROM_ABI _Tm __convert_to_tm(const _ChronoT& __value) {
 #  endif
 
   if constexpr (__is_time_point<_ChronoT>) {
-    if constexpr (same_as<typename _ChronoT::clock, chrono::system_clock>)
-      return std::__convert_to_tm<_Tm>(__value);
-#  if _LIBCPP_HAS_TIME_ZONE_DATABASE && _LIBCPP_HAS_FILESYSTEM && _LIBCPP_HAS_LOCALIZATION
-#    if _LIBCPP_HAS_EXPERIMENTAL_TZDB
-    else if constexpr (same_as<typename _ChronoT::clock, chrono::utc_clock>)
-      return std::__convert_to_tm<_Tm>(__value);
-    else if constexpr (same_as<typename _ChronoT::clock, chrono::tai_clock>)
-      return std::__convert_to_tm<_Tm>(__value);
-#    endif // _LIBCPP_HAS_EXPERIMENTAL_TZDB
-#  endif   // _LIBCPP_HAS_TIME_ZONE_DATABASE && _LIBCPP_HAS_FILESYSTEM && _LIBCPP_HAS_LOCALIZATION
-    else if constexpr (same_as<typename _ChronoT::clock, chrono::file_clock>)
+    if constexpr (same_as<typename _ChronoT::clock, chrono::file_clock>)
       return std::__convert_to_tm<_Tm>(_ChronoT::clock::to_sys(__value));
     else if constexpr (same_as<typename _ChronoT::clock, chrono::local_t>)
       return std::__convert_to_tm<_Tm>(chrono::sys_time<typename _ChronoT::duration>{__value.time_since_epoch()});
     else
+      // Note that some clocks have specializations __convert_to_tm for their
+	  // time_point. These don't need to be added here. They do not trigger
+	  // this assert.
       static_assert(sizeof(_ChronoT) == 0, "TODO: Add the missing clock specialization");
   } else if constexpr (chrono::__is_duration_v<_ChronoT>) {
     // [time.format]/6

@github-actions
Copy link

github-actions bot commented Mar 19, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM with nit.

Comment on lines 151 to 153
// Note that some clocks have specializations __convert_to_tm for their
// time_point. These don't need to be added here. They do not trigger
// this assert.
Copy link
Member

Choose a reason for hiding this comment

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

Let's use braces with the else -- the current layout is confusing.

For certain time_points there are specializations of __convert_to_tm.
This means the non-specialized version is never called. This means
some of the `if constexpr` will never be true. These are removed.

Since there is a `static_assert` accidental removal of the
specialization will make the code ill-formed.
@mordante mordante force-pushed the review/removes_dead_code branch from b401d04 to 18d525b Compare March 20, 2025 16:58
@mordante mordante merged commit 7be243a into llvm:main Mar 21, 2025
81 checks passed
@mordante mordante deleted the review/removes_dead_code branch March 21, 2025 17:02
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.

3 participants