Variant[Variant]

[Ali Çehreli]

On 08/27/2013 03:53 AM, Jason den Dulk wrote:

 > I uploaded a new version with a main routine. So you know, I am using
 > DMD 2.063.2 on Fedora 15.
 >
 > The code should compile with "dmd serialize.d".

And it's here:

   http://jaypha.com.au/serialize.d

 > I since realized that I was not dealing with a Variant[Variant] but a
 > Variant(Variant[Variant]). It didn't seem to like having "[Variant]"
 > called upon it.
 >
 > After extracting it to a V[V] (called tcd1_vn), I noticed something else.
 >
 > Doing a foreach on it,
 >
 >    foreach (_a_, _b_;tcd1_vn)
 >    {
 >      writeln(_a_.type(),",",_b_.type());
 >      writeln(_a_.get!(string),",",_b_.get!(long));
 >    }
 >
 > gives this:
 >
 >    immutable(char)[],long
 >    a,1
 >    immutable(char)[],long
 >    b,2
 >    immutable(char)[],long
 >    c,3
 >
 > So it appears that Variant("a") is a key, but
 >
 >    assert(Variant("a") in tcd1_vn);
 >
 > fails. Any ideas?

It is definitely a bug. I spent a lot of time on this and figured it out 
just before giving up. :) (Actually I figured it out *after* giving up. :p)

The signature of VariantN.toHash() does not match D's expectation so it 
is not considered at all. As a result, the default toHash for structs 
gets called, which happens to hash by the members of the struct. The 
problem is, being a variant type, the members of VariantN are a function 
pointer and the storage:

struct VariantN(size_t maxDataSize, AllowedTypesX...)
{
// ...
     ptrdiff_t function(OpID selector, ubyte[size]* store, void* data) fptr
         = &handler!(void);
     union
     {
         ubyte[size] store;
         // conservatively mark the region as pointers
         static if (size >= (void*).sizeof)
             void* p[size / (void*).sizeof];
     }
// ...
}

Since 'store' is just a ubyte[] array, the default toHash for structs 
cannot hash it as strings.

VariantN.toHash's signature must be changed accordingly and Phobos must 
be recompiled:

     // WARNING: Unnecessarily slow because type.getHash() is not nothrow.
     size_t toHash() const nothrow @safe
     {
         try {
             return type.getHash(&store);

         } catch (Exception) {
             return 0;
         }
     }

The original toHash was this:

     size_t toHash()
     {
         return type.getHash(&store);
     }

The other issue is, the compiler did not warn me until I added a const 
to the signature. Try to compile this:

     size_t toHash() const
     {
         return type.getHash(&store);
     }

./phobos/std/variant.d(822): Warning: toHash() must be declared as 
extern (D) size_t toHash() const nothrow @safe, not const @trusted ulong()

The same warning should be given to the naked signature 'size_t 
toHash()' as well. (I remember bearophile warning about such problems.)

So, VariantN is broken. It cannot be used as an AA key correctly, at 
least when it stores a string. To make matters worse, testing with 
Variants that are initialized by literal strings works because the 
compiler optimizes the same literal strings.

 > Thanks again for helping. I have been reading your online book. I have
 > found it quite helpful.

You are very kind. :) However, as this issue proves, the book is 
outdated in some parts. At least there is a note for me to update the 
toHash signatures in the book: :)

 
https://code.google.com/p/ddili/source/browse/trunk/src/ders/d.en/object.d#681

So, I will get to this issue eventually. :-/

May I ask you or somebody else to create a bug about VariantN.toHash not 
being considered at all. Thank you! :)

Ali