Skip to content
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

Test that default constructed objects are zero #529

Closed
wants to merge 1 commit into from

Conversation

NAThompson
Copy link
Contributor

In a generic context, we often need to compare arithmetic types to zero. However, mp-units types cannot be default constructed from (say) double, so we cann write (e.g.)

if (x < T(0)) {
  // ...
}

Instead, we have to use

constexpr auto ZERO = T{};
if (x < ZERO) {
   // ...
}

This begs the question: Do we have a guarantee that ZERO==0.0? Add a unit test that verifies this behavior.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Dec 16, 2023

I think these should go before

// construction from a value

// construction from a value

And test that (0 * m).numerical_value_in(m) == quantity<m, int>::zero().
The earlier tests already check that zero returns 0 or 0.0.

@JohelEGP
Copy link
Collaborator

I was under the impression that

static_assert(std::is_trivially_default_constructible_v<quantity<isq::length[m]>>);
static_assert(std::is_trivially_copy_constructible_v<quantity<isq::length[m]>>);
static_assert(std::is_trivially_move_constructible_v<quantity<isq::length[m]>>);
static_assert(std::is_trivially_copy_assignable_v<quantity<isq::length[m]>>);
static_assert(std::is_trivially_move_assignable_v<quantity<isq::length[m]>>);
static_assert(std::is_trivially_destructible_v<quantity<isq::length[m]>>);

static_assert(std::is_trivially_copyable_v<quantity<isq::length[m]>>);

implied that.
But thinking about it some more, I'm not too convinced that they guarantee it.

@NAThompson
Copy link
Contributor Author

I think these should go before

// construction from a value

// construction from a value

I of course have no objection to doing this, but then I'd have to bring in the math.h header into quantity_test.cpp. Feels like it would decrease the orthogonality of the test suite.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Dec 16, 2023

quantity_test.cpp already tests that specializations of quantity model std::default_initializable.
It also tests quantity::zero.
You shouldn't need math.h to connect both in quantity_test.cpp.

In a generic context, we often need to compare arithmetic types to zero.
However, mp-units types cannot be default constructed from (say) `double`,
so we cann write (e.g.)

```cpp
if (x < T(0)) {
  // ...
}
```

Instead, we have to use

```cpp
constexpr auto ZERO = T{};
if (x < ZERO) {
   // ...
}
```

This begs the question: Do we have a guarantee that `ZERO==0.0`?
Add a unit test that verifies this behavior.
@NAThompson
Copy link
Contributor Author

NAThompson commented Dec 16, 2023

It appears that it is unclear what this PR accomplishes-if anything. I think this can safely be closed.

If anyone finds compelling value in it I'll reopen.

@NAThompson NAThompson closed this Dec 16, 2023
@mpusz
Copy link
Owner

mpusz commented Dec 17, 2023

However, mp-units types cannot be default constructed from (say) double, so we cann write (e.g.)

if (x < T(0)) {
  // ...
}

You actually do not mean a default initialization but value/zero-initalization (https://en.cppreference.com/w/cpp/language/zero_initialization) here.

In generic code, you can always write:

if (x < T{}) {
  // ...
}

A more detailed discussion on this subject can be found at https://wg21.link/p2982R1#comparison-against-zero. We will discuss this in the Committee and check if there is a guidance in any direction.

@NAThompson NAThompson deleted the test_zero branch December 17, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants