Skip to content

Conversation

StephanTLavavej
Copy link
Member

  • Fix copy-paste damage in formatter.tai_time.pass.cpp.
    • Comparing the #if to the #else case, it's clear that this half-copied check should be removed.
  • Mark is_steady as [[maybe_unused]].
    • It's only used within LIBCPP_STATIC_ASSERT.

Comparing the `#if` to the `#else` case, it's clear that this half-copied check should be removed.
It's only used within `LIBCPP_STATIC_ASSERT`.
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 22, 2025 07:25
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 22, 2025

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes
  • Fix copy-paste damage in formatter.tai_time.pass.cpp.
    • Comparing the #if to the #else case, it's clear that this half-copied check should be removed.
  • Mark is_steady as [[maybe_unused]].
    • It's only used within LIBCPP_STATIC_ASSERT.

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

3 Files Affected:

  • (modified) libcxx/test/std/time/time.clock/time.clock.gps/types.compile.pass.cpp (+5-5)
  • (modified) libcxx/test/std/time/time.clock/time.clock.tai/types.compile.pass.cpp (+5-5)
  • (modified) libcxx/test/std/time/time.syn/formatter.tai_time.pass.cpp (-3)
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 af95c5fe73402..006ad9a2d243e 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,11 @@
 #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;
-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;
+[[maybe_unused]] constexpr bool is_steady = std::chrono::gps_clock::is_steady;
 
 // Tests the values. part of them are implementation defined.
 LIBCPP_STATIC_ASSERT(std::same_as<rep, std::chrono::utc_clock::rep>);
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 ddb057333f49a..a7123bc3e0b5c 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,11 @@
 #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;
-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;
+[[maybe_unused]] constexpr bool is_steady = std::chrono::tai_clock::is_steady;
 
 // Tests the values. part of them are implementation defined.
 LIBCPP_STATIC_ASSERT(std::same_as<rep, std::chrono::utc_clock::rep>);
diff --git a/libcxx/test/std/time/time.syn/formatter.tai_time.pass.cpp b/libcxx/test/std/time/time.syn/formatter.tai_time.pass.cpp
index 7ca088cc6e8f4..6a1d5bde44d35 100644
--- a/libcxx/test/std/time/time.syn/formatter.tai_time.pass.cpp
+++ b/libcxx/test/std/time/time.syn/formatter.tai_time.pass.cpp
@@ -268,9 +268,6 @@ static void test_valid_values_day() {
         lfmt,
         cr::tai_seconds(1'613'259'090s)); // 23:31:30 TAI Friday, 13 February 2009
 
-  // Use the global locale (fr_FR)
-  check(SV("%d='01'\t%Od='01'\t%e=' 1'\t%Oe=' 1'\n"),
-        lfmt,
 #else // defined(_WIN32) || defined(__APPLE__) || defined(_AIX) || defined(__FreeBSD__)
   check(loc,
         SV("%d='01'\t%Od='一'\t%e=' 1'\t%Oe='一'\n"),

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

As the is_steady is tested only once (ditto tai_clock::is_steady) and doesn't seem necessary for twice, should we just remove it and write LIBCPP_STATIC_ASSERT(std::chrono::gps_clock::is_steady == false); instead?
CC @mordante.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that time.clock.utc/types.compile.pass.cpp is also affected. (I missed gps and tai in my previous #131787.)

Copy link
Member

Choose a reason for hiding this comment

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

I agree it's cleaner, but there are indeed more clocks affected. I've created #132546 to look into this.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes, LGTM!

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;
Copy link
Member

Choose a reason for hiding this comment

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

I agree it's cleaner, but there are indeed more clocks affected. I've created #132546 to look into this.

@StephanTLavavej StephanTLavavej merged commit 51aab96 into llvm:main Mar 22, 2025
84 checks passed
@StephanTLavavej StephanTLavavej deleted the more-test-fixes branch March 22, 2025 13:38
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