Skip to content

[NFC][libc++][chrono] Removes dead code. #132104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2025

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

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