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

Chore: Centralize Strong<> type unwrapping #4170

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

reneme
Copy link
Collaborator

@reneme reneme commented Jul 1, 2024

Closes #4029.

This introduces wrap_strong_type<> and unwrap_strong_type() that opportunistically wrap a given object into a strong type or extract the value from a strong type. It is meant to be used in generic contexts where we may or may not have to deal with a strong type. Examples are the load/store methods and checked_cast_to.

Another new helper is strong_type_wrapped_type<>. Given a potentially strong-typed type, this figures out the underlying type. That is quite useful to build generic functions that take both a bare value or a strong-type of the bare value. Like so:

template <typename T>
   requires std::signed_integral<strong_type_wrapped_type<T>>
void foo(T signed_int_that_may_be_wrapped_by_a_strong_type) {
   const auto bare_signed_int = unwrap_strong_type(signed_int_that_may_be_wrapped_by_a_strong_type);
   /* ... work with bare integer ... */
}

// For future reference: I would have loved to define a higher-order concept
// instead of `std::signed_integral<strong_type_wrapped_type<T>>`.
// In imaginary syntax that might have looked like that:

template <typename T, template <typename> typename SomeUnaryConcept>
concept maybe_strong_type_of =
   (concepts::strong_type<T> && SomeUnaryConcept<typename T::wrapped_type>)
   ||
   (!concepts::strong_type<T> && SomeUnaryConcept<T>);

// The function above could then be defined as:
template <maybe_strong_type_of<std::signed_integral> T>
void foo(T signed_int_that_may_be_wrapped_by_a_strong_type);

Many other generic locations that currently use byte-buffer-ish strong types don't have to rely on this, as those strong types masquerade as byte buffers and can be used directly. This is mostly targeting int-based strong types where we really need access to the underlying object for low-level operations.

We may or may not add this to 3.5.0 as it is a low-level refactoring. I'm quite confident in the CI coverage of this, though.

@reneme reneme added this to the Botan 3.5.0 milestone Jul 1, 2024
@reneme reneme self-assigned this Jul 1, 2024
Comment on lines 175 to 206
template <unsigned_integralish T>
using wrapped_type = decltype([] {
if constexpr(std::is_enum_v<T>) {
return std::underlying_type_t<T>{};
} else {
return Botan::strong_type_wrapped_type<T>{};
}
}());

template <unsigned_integralish InT>
constexpr auto unwrap_strong_type_or_enum(InT t) {
if constexpr(std::is_enum_v<InT>) {
// TODO: C++23: use std::to_underlying(in) instead
return static_cast<std::underlying_type_t<InT>>(t);
} else {
return Botan::unwrap_strong_type(t);
}
}

template <unsigned_integralish OutT, std::unsigned_integral T>
constexpr auto wrap_strong_type_or_enum(T t) {
if constexpr(std::is_enum_v<OutT>) {
return static_cast<OutT>(t);
} else {
return Botan::wrap_strong_type<OutT>(t);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an extra layer of abstraction to make load/store work with integer-enums as well. I feel that this would be useful for the other usage locations as well, so in theory we could also promote that into the implementation in strong_type.h. As a result, the checked_cast_to would then automagically work with enums as well.

@reneme reneme force-pushed the chore/strongtype_unwrapping branch from a6b9ed7 to 2addfcf Compare July 1, 2024 16:35
Copy link
Collaborator

@FAlbertDev FAlbertDev left a comment

Choose a reason for hiding this comment

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

LGTM. I admit I mostly looked into the tests to see if every case was covered 😝
The only thing I can think of otherwise is support for strong types of strong types. In this case, loadstore does not accept these strong-ception types. But I don't think we want to support this anyway.

@reneme
Copy link
Collaborator Author

reneme commented Jul 2, 2024

Yeah, I did wonder about that. But rejected the idea of strong type-ception as useless. Perhaps we should even statically check that in the strong type implementation.

@randombit randombit modified the milestones: Botan 3.5.0, Botan 3.6.0 Jul 6, 2024
@randombit
Copy link
Owner

Bumping this to 3.6, with just 2 days until 3.5.0 release I'd rather not change anything except where we must.

@reneme reneme force-pushed the chore/strongtype_unwrapping branch from 2addfcf to 0f465bd Compare July 10, 2024 14:06
Comment on lines +50 to +56
constexpr T& get() & { return m_value; }

const T& get() const { return m_value; }
constexpr const T& get() const& { return m_value; }

constexpr T&& get() && { return std::move(m_value); }

constexpr const T&& get() const&& { return std::move(m_value); }
Copy link
Collaborator Author

@reneme reneme Jul 10, 2024

Choose a reason for hiding this comment

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

This was inspired by #4175 (comment) and it simplifies the code of strong_type_unwrap() significantly. In a sense, that it can now rely on overloading instead of explicitly std::move()ing based on the type of object passed in.

@coveralls
Copy link

Coverage Status

coverage: 92.045% (+0.3%) from 91.699%
when pulling 0f465bd on Rohde-Schwarz:chore/strongtype_unwrapping
into 45be74e on randombit:master.

@reneme reneme merged commit bb9c069 into randombit:master Jul 10, 2024
39 checks passed
@reneme reneme deleted the chore/strongtype_unwrapping branch July 10, 2024 15:28
reneme added a commit to Rohde-Schwarz/botan that referenced this pull request Jul 16, 2024
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.

Centralize 'integralish' concept and strong-type unwrapping
4 participants