-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Use LIBCPP_STATIC_ASSERT in chrono test
#132566
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
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-libcxx Author: None (PaulXiCao) ChangesCloses #132546. Full diff: https://github.com/llvm/llvm-project/pull/132566.diff 3 Files Affected:
diff --git a/libcxx/test/std/time/time.clock/time.clock.gps/types.compile.pass.cpp b/libcxx/test/std/time/time.clock/time.clock.gps/types.compile.pass.cpp
index 006ad9a2d243e..81604b6e8be7f 100644
--- a/libcxx/test/std/time/time.clock/time.clock.gps/types.compile.pass.cpp
+++ b/libcxx/test/std/time/time.clock/time.clock.gps/types.compile.pass.cpp
@@ -36,11 +36,10 @@
#include "test_macros.h"
// class gps_clock
-using rep = std::chrono::gps_clock::rep;
-using period = std::chrono::gps_clock::period;
-using duration = std::chrono::gps_clock::duration;
-using time_point = std::chrono::gps_clock::time_point;
-[[maybe_unused]] constexpr bool is_steady = std::chrono::gps_clock::is_steady;
+using rep = std::chrono::gps_clock::rep;
+using period = std::chrono::gps_clock::period;
+using duration = std::chrono::gps_clock::duration;
+using time_point = std::chrono::gps_clock::time_point;
// Tests the values. part of them are implementation defined.
LIBCPP_STATIC_ASSERT(std::same_as<rep, std::chrono::utc_clock::rep>);
@@ -52,7 +51,7 @@ static_assert(std::same_as<period, std::ratio<period::num, period::den>>);
static_assert(std::same_as<duration, std::chrono::duration<rep, period>>);
static_assert(std::same_as<time_point, std::chrono::time_point<std::chrono::gps_clock>>);
-LIBCPP_STATIC_ASSERT(is_steady == false);
+LIBCPP_STATIC_ASSERT(std::chrono::gps_clock::is_steady == false);
// typedefs
static_assert(std::same_as<std::chrono::gps_time<int>, std::chrono::time_point<std::chrono::gps_clock, int>>);
diff --git a/libcxx/test/std/time/time.clock/time.clock.tai/types.compile.pass.cpp b/libcxx/test/std/time/time.clock/time.clock.tai/types.compile.pass.cpp
index a7123bc3e0b5c..b411b1a13acf6 100644
--- a/libcxx/test/std/time/time.clock/time.clock.tai/types.compile.pass.cpp
+++ b/libcxx/test/std/time/time.clock/time.clock.tai/types.compile.pass.cpp
@@ -36,11 +36,10 @@
#include "test_macros.h"
// class tai_clock
-using rep = std::chrono::tai_clock::rep;
-using period = std::chrono::tai_clock::period;
-using duration = std::chrono::tai_clock::duration;
-using time_point = std::chrono::tai_clock::time_point;
-[[maybe_unused]] constexpr bool is_steady = std::chrono::tai_clock::is_steady;
+using rep = std::chrono::tai_clock::rep;
+using period = std::chrono::tai_clock::period;
+using duration = std::chrono::tai_clock::duration;
+using time_point = std::chrono::tai_clock::time_point;
// Tests the values. part of them are implementation defined.
LIBCPP_STATIC_ASSERT(std::same_as<rep, std::chrono::utc_clock::rep>);
@@ -52,7 +51,7 @@ static_assert(std::same_as<period, std::ratio<period::num, period::den>>);
static_assert(std::same_as<duration, std::chrono::duration<rep, period>>);
static_assert(std::same_as<time_point, std::chrono::time_point<std::chrono::tai_clock>>);
-LIBCPP_STATIC_ASSERT(is_steady == false);
+LIBCPP_STATIC_ASSERT(std::chrono::tai_clock::is_steady == false);
// typedefs
static_assert(std::same_as<std::chrono::tai_time<int>, std::chrono::time_point<std::chrono::tai_clock, int>>);
diff --git a/libcxx/test/std/time/time.clock/time.clock.utc/types.compile.pass.cpp b/libcxx/test/std/time/time.clock/time.clock.utc/types.compile.pass.cpp
index 8cb3d78d97f52..292e2c47c02b6 100644
--- a/libcxx/test/std/time/time.clock/time.clock.utc/types.compile.pass.cpp
+++ b/libcxx/test/std/time/time.clock/time.clock.utc/types.compile.pass.cpp
@@ -36,11 +36,10 @@
#include "test_macros.h"
// class utc_clock
-using rep = std::chrono::utc_clock::rep;
-using period = std::chrono::utc_clock::period;
-using duration = std::chrono::utc_clock::duration;
-using time_point = std::chrono::utc_clock::time_point;
-[[maybe_unused]] constexpr bool is_steady = std::chrono::utc_clock::is_steady;
+using rep = std::chrono::utc_clock::rep;
+using period = std::chrono::utc_clock::period;
+using duration = std::chrono::utc_clock::duration;
+using time_point = std::chrono::utc_clock::time_point;
// Tests the values. Some of them are implementation-defined.
LIBCPP_STATIC_ASSERT(std::same_as<rep, std::chrono::system_clock::rep>);
@@ -52,7 +51,7 @@ static_assert(std::same_as<period, std::ratio<period::num, period::den>>);
static_assert(std::same_as<duration, std::chrono::duration<rep, period>>);
static_assert(std::same_as<time_point, std::chrono::time_point<std::chrono::utc_clock>>);
-LIBCPP_STATIC_ASSERT(is_steady == false);
+LIBCPP_STATIC_ASSERT(std::chrono::utc_clock::is_steady == false);
// typedefs
static_assert(std::same_as<std::chrono::utc_time<int>, std::chrono::time_point<std::chrono::utc_clock, int>>);
|
mordante
left a comment
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.
Thanks for working on this! I think we need a different approach as described in the review comment.
I also would like to update the existing clocks in other test, some of these are pre-C++20 so can't use the std::same_as<bool> decltype(auto) method used here.
Updating the other clocks was my main motivation to file a bug report instead of requesting fixes in the original PR.
| static_assert((std::is_same<C::duration, C::time_point::duration>::value), ""); | ||
| static_assert(C::is_steady || !C::is_steady, ""); | ||
| static_assert(std::is_same<decltype(C::is_steady), const bool>::value, ""); | ||
| test(std::chrono::high_resolution_clock::is_steady); |
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.
Does someone know what the line test(...::is_steady); is supposed to test?
Incase it is just testing that the member exists it is probably already enough to have the static_assert line above?
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.
The is_steady tests whether the clock is steady. This means the clock has no time jumps. For some clocks the Standard requires them to be steady, for others it's implementation defined. The [time.clock] has the information. When unspecified we need a LIBCPP_ASSERT to validate the value libc++ uses.
Closes #132546.