-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesFor certain time_points there are specializations of __convert_to_tm. This means the non-specialized version is never called. This means some of the Since there is a Full diff: https://github.com/llvm/llvm-project/pull/132104.diff 1 Files Affected:
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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit.
// 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. |
There was a problem hiding this comment.
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.
b401d04
to
18d525b
Compare
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.