Allow &synchronized with static length fields and not just literals

[sethhall]

Some protocols have static length fields before the literal where we might use the &synchronize attribute but currently spicy only allows literals before the literal where &synchronize is used.

Here is a toy parser that should illustrate what I would like to be able to do...

module test;

public type PDUs = unit {
    : (PDU &synchronize)[];
};

type PDU = unit {
    len: uint8;
    sub: subPDU &size=self.len;
};

type subPDU = unit {
    typ: /this|that/ &synchronize;
    switch (self.typ) {
        /this/ -> eight: uint8;
        /that/ -> thirty_two: uint32;
        *      -> : bytes &eod;
    };
};

This currently give this error....

[error] ../test13.spicy:9:10-9:14: &synchronize cannot be used on field, look-ahead contains non-literals
[error] spicyc: aborting after errors

[rsmmr]

This example should be doable, but one note to double-check: I don't think it'd work with look-ahead but would need to do a direct match on the literal. I.e, it wouldn't work if there were more units each having some static length fields before their literals. That's because for look-ahead, the candidate literals all need to start at the same location. However, it might be workable if all static length fields are the same size, need to see.

That ok?

[sethhall]

I'm not sure I understand what you're asking? It sounds like I may need to double check my example to see how close it is to the scenario I'm encountering in the parser. I was aiming for a kind of similar parser here but cut down as far as I could.

[rsmmr]

Here is a version of what I meant. In addition to your example,I think (hope ...) we could also support the following:

module test;

public type PDUs = unit {
    : (PDU &synchronize)[];
};

type PDU = unit {
    switch {
        -> a: A;
        -> b: B;
    };
};

type A = unit {
    len: uint8;
    sub: subA &size=self.len;
};

type B = unit {
    len: uint8; # (*)
    sub: subB &size=self.len;
};

type subA = unit {
    typ: /this/;
};

type subB = unit {
    typ: /that/;
};

However, if we replace the uint8 at (*) with uint64, then we couldn't support it anymore, because now the look-ahead symbols we're looking for to recognize subA/subB no longer start at the same spot.

(This might actually be even more relevant for normal look-ahead parsing than error recovery, but it's all the same machinery.)

[sethhall]

Ah, thanks for the explanation. I'm not sure that if that different width header thing would bite us (the 8/64 thing you pointed out) but my initial gut feeling is that eventually it would.

I did just create a work around for this issue, but it's truly awful although perhaps better than the existing attempt.

module test;
import spicy;

public type PDUs = unit {
    : (PDU &synchronize)[];
};

type PDU = unit {
    len: /./ &convert=$$.to_uint(spicy::ByteOrder::Big);
    sub: subPDU &size=self.len;
};

type subPDU = unit {
    typ: /this|that/ {
        confirm;
    }
    switch (self.typ) {
        b"this" -> eight: uint8;
        b"that" -> thirty_two: uint32;
        *      -> : bytes &eod;
    };
};

The approach might not be all that surprising, this just can't be any sort of long term solution. :) I suspect I will be prodding more about supporting the 8/64 issue though, that does seem fairly important as I think about it more. Unfortunately I don't have a good sense yet of how hard that would be to support.

[rsmmr]

I suspect I will be prodding more about supporting the 8/64 issue though, that does seem fairly important as I think about it more.

I'm starting to think maybe we could support it, though it might not fit well with the current machinery. Need to think more about how we would implement it.

Btw, this started out by you pointing out that that the docs are explaining a recovery mechanism that's not implemented. I believe what this ticket describes is a different case though: the case in the docs just jumps forward to a certain offset, it doesn't necessarily need to find a literal there then.

[sethhall]

Heh, yes. Just having that documented mechanism work would be great. I suppose I shouldn't distract in this ticket by getting greedy and asking for more. :)