Re: [PATCH 2/3] variants: minor improvements

[Akim Demaille]

Le 4 janv. 2013 à 13:55, Theophile Ranquet <address@hidden> a écrit :

> * data/variant.hh (swap): More asserts can't hurt.
> (variant): Prohibit copy construction.
> 
> diff --git a/data/variant.hh b/data/variant.hh
> index 1ebcfe8..afa825e 100644
> --- a/data/variant.hh
> +++ b/data/variant.hh
> @@ -165,6 +165,8 @@ m4_define([b4_variant_define],
>     inline void
>     swap (variant<S>& other)
>     {]b4_parse_assert_if([
> +      YYASSERT (built);
> +      YYASSERT (other.built);
>       YYASSERT (tname == other.tname);])[
>       std::swap (as<T>(), other.as<T>());]b4_parse_assert_if([
>       std::swap (built, other.built);

This seems fishy: if both are asserted to be true, why
swapping them?  Maybe the function was intended to swap
between constructed and not-constructed, if it does make
sense.

In either way, more comments to document the function would
be needed.

> @@ -208,6 +210,11 @@ m4_define([b4_variant_define],
>       abort ();
>     }
> 
> +    variant (const self_type&)
> +    {
> +      abort ();
> +    }
> +
>   private:
>     /// A buffer large enough to store any of the semantic values.
>     /// Long double is chosen as it has the strongest alignment
> -- 
> 1.8.1
> 
>