[erlang-questions] Extending term external format to support shared substructures

[Matthew Dempsky]

On Tue, Mar 31, 2009 at 2:46 AM, Bjorn Gustavsson <bgustavsson@REDACTED> wrote:
> Thanks for the patch. Even though it looks fine, we will not include until
> the code for encoding a term has been written.

That's reasonable.

> Your fix for a zero length LIST_EXT doesn't seem to be correct, though.
> Have you tried:
>
> binary_to_term(<<131,104,2,108,0,0,0,0,106,100,0,1,97>>).

Here's what I get with my patched R13A:

    > binary_to_term(<<131,104,2,108,0,0,0,0,106,100,0,1,97>>).
    {[],a}

Do you get something different?  Is this not what you think it should be?

> My correction for this problem will appear in the next R13B
> snapshot (hopefully tomorrow) at:

Great.

> (It is also good idea to use a snapshot as a base for further patches,
> as I have eliminated the deep recursion in term_to_binary/1.)

Got it.

> To make sure that all terms are properly nested. We try to do as much error
> checking as possible while calculating the size.

Sorry, I still don't see the benefit.  E.g., what better error
checking do you have right now than if in the clause for
SMALL_TUPLE_EXT you were to replace

    ESTACK_PUSH(s, terms);
    terms = *ep++;
    heap_size += terms + 1;

with

    n = *ep++;
    terms += n;
    heap_size += n + 1;

?  With the exception of overflowing the 'terms' counter (which can be
easily resolved by changing it to a 64-bit integer), I don't see how
the former ends up being any stricter than the latter.

Also, now that I look at it again, it looks like there's a memory leak
when SKIP, SKIP2, or CHKSIZE fail (and also the 'first' check that I
added in the DICTIONARY clause in my patch, because I based it on
these), because they return without calling DESTROY_ESTACK.