Add CT::Option

[randombit]

GH #4172

[coveralls]

coverage: 91.729% (-0.2%) from 91.932%
when pulling 8d376b0 on jack/ct-option
into 3657a72 on master.

[reneme]

Nice idea!

Frankly, I would suggest to stay with the STL's nomenclature of things, though. I.e.

• unwrap() -> value()
• unwrap_or() -> value_or()
• is_some() -> has_value()
• map() -> transform() (C++23: std::optional<>::transform)

[reneme]

Probably a good idea to scope this to types that actually provide this method. And also make the copy of other visible via the interface (to optimize usage with rvalue refs for instance):

Suggested change

	/// Returns either the inner value or the alternative, in constant time
	T unwrap_or(const T& other) const {
	T r = other;
	r.conditional_assign(m_is_some, m_value);
	return r;
	}
	/// Returns either the inner value or the alternative, in constant time
	T unwrap_or(T other) const
	requires concepts::ct_conditional_assignable<T>
	{
	other.conditional_assign(m_is_some, m_value);
	return other;
	}

... and the concept itself:

namespace concepts {

template <typename T>
concept ct_conditional_assignable = requires(T lhs, const T& rhs, Choice c) { lhs.conditional_assign(c, rhs); };

}  // namespace concepts

}

... if you wanted to, you could even provide a variable time overload if the concept isn't fulfilled. Though, I'd consider this fairly reckless, as it would potentially fool the user if their type doesn't model the concept (perhaps even subtly inadvertantly).

[randombit]

I'd be quite against a variable time overload. That said it would be nice if we could handle this more generically, ie CT::Option<uint32_t> is entirely a sensible thing, and it's quite possible to implement unwrap_or in constant time as well, but requires additional help. I'll ponder this.

[randombit]

Nice way of expressing the conditional assignment btw.

I made a change such that value_or can handle either types which explicitly provided an appropriate conditional_assign, and for unsigned integral types which make use of CT::Mask to perform the selection.

[coveralls]

coverage: 91.74% (+0.01%) from 91.726%
when pulling 07dbfb2 on jack/ct-option
into 7fad1d2 on master.

[randombit]

I moved the CT tests to a new file as test_utils.cpp is getting quite large. The Mask and Choice tests are unchanged

[reneme]

Perhaps also constexpr the remaining member functions. I don't see why any of them wouldn't be constexpr.

[reneme]

Should that really return a copy of m_value? Maybe:

Suggested change

	/// Either returns the value or throws an exception
	T value() const {
	BOTAN_STATE_CHECK(m_is_some.as_bool());
	return m_value;
	}
	/// Either returns the value or throws an exception
	const T& value() const {
	BOTAN_STATE_CHECK(m_is_some.as_bool());
	return m_value;
	}

... for completeness, std::optional defines this both for lvalue and rvalue this. Not sure we really need that, though. It just optimizes cases where a temporary is passed on.

takes_an_Option_by_value(returns_an_option_by_value().value())

[randombit]

I suppose I was thinking of Option as largely for holding types that are cheap to copy (integers, simple arrays, etc) where this wouldn't matter, but yeah no reason not to return a reference here, CT::Option<vector<uint8_t>> is entirely sensible and might indeed be something worth using in PK decryption or the like.

[reneme]

When we foresee to use it with std::vector<uint8_t> it would actually make sense to implement the lvalue/rvalue overloads that std::optional<> implements. In fact, this just made me re-think the strong_type_unwrap() helper: https://github.com/randombit/botan/pull/4170/files#r1672372281

[randombit]

Reading https://stackoverflow.com/questions/24824432/what-is-use-of-the-ref-qualifier-const I am unconvinced by const && and don't really see benefit. I'm going to merge this as is and if/when Option of non-trivally-copyable types comes up we can consider the best course.

[reneme]

Perhaps its worth to reconsider this before merging.