Re: [boost] markable -- informal review request

Subject: Re: [boost] markable -- informal review request
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2016-09-20 19:48:51

On 21 Sep 2016 12:32 am, "Giovanni Piero Deretta" <gpderetta_at_[hidden]>
wrote:
>
>
> On 20 Sep 2016 12:04 pm, "Andrzej Krzemienski" <akrzemi1_at_[hidden]> wrote:
> >
> [...].
>
> >
> > One could consider other alternatives like "magic_val", but they are not
> > much better, and do not transform into adjectives and verbs that easily
> > ("magic_valued"?).
> >
> > Does that sound convincing?
> >
>
> Maybe.
>
> More seriously, yes please submit it to boost, I have seen reinvented (a
few by myself) this too many times. A few comments:
>
> * Consider, maybe, a different name (OK, OK I'll stop).
> * The first template parameter should be the stored T itself, pass the
policy as a separate optional parameter.
> * Pick a sensible default policy. I like nan for floats, otherwise value
initialisation is a good default. For integers is easy to have a policy
specifying the value inline.
> * to avoid the bag of functions issue with policies, consider using ADL
for customisation and use the stateless policy just to drive the lookup.
> * The policy itself can be used for tagging, no need for an extra
parameter.
> * Markable should be trivially copyable, assignable and destructible when
the underlying T is, at least when not using the pod storage policy. This
might be already the case.
> * Add flatMap?
>
> That's all for now,
>
Oh, I forgot: from the docs:

"It is impossible to alter the "contained value" through the access to
function value[...]

With the assignment being prevented, we could get the following surprising
behaviour:

marked<mp_int<int, -1>> oi {1};

oi.value() = (int)some_value; // < suppose it was allowed

assert (oi.has_value()) // < might fail!

"

I don't find it more surprising than this:

marked<mp_int<int, -1>> oi {1};
oi = marked<mp_int<int, -1>>(some_value);
assert (oi.has_value()) // < might fail!

This should be allowed as long as the storage type is really T ( in fact I
suggest in this case making the underlying field public and allowing
aggregate initialisation).