[Cbor] Re: draft-ietf-cbor-edn-literals-10 implementation notes

[Joe Hildebrand <[e-mail]>]

Note to Rohan: look for the part about PR#49.

(elided places where we've come to conclusions)


> On Aug 17, 2024, at 10:58 AM, Carsten Bormann <[e-mail]> wrote:
> 
>>>> - s4.1 It should be an error to mix app-strings and strings, or app-strings of multiple types.  The text at the end of the section starting with "Some of the strings may be app-strings..." is not proscriptive enough.
>>> 
>>> This is a reasonable conclusion when you just look at the set of application-extensions that are in the document, but it breaks down when constructing strings from components that can be described by app-strings (something like a JWT that mixes base64-encoded strings with delimiters such as “.” and “~”).
>>> (The example would need to build the base64-encoded parts using application-oriented literals, for which we don’t have an application-oriented literal — b64 goes the other way around.  I find it quite likely that we’ll have application-oriented literals for JWT and CWT over time.)
>> 
>> Can we at least say that the output type of the app-strings need to be compatible enough to concatenate?
> 
> The last bullet in 4.1 tries to do that.

>> Take your 'f' approach from above; what would f'2' + f'3' mean?
> 
> " • Some of the strings may be app-strings. If the type of the app-string is an actual string, joining of chunked strings occurs as with directly notated strings; otherwise the occurrence of more than one app-string or an app-string together with a directly notated string cannot be processed.”
> 
> I read the answer as “cannot be processed”, which was a way to say “not allowed”.  Wording fixes appreciated.

Maybe after reading G.4 of 8610 several times I think I understand.  However, I feel strongly that all of G.4 needs to be pulled into this document and made more clear, with "+" in the correct places.  I'm willing to help with that effort.

This is an example of why the reliance on extending 8610 does not work for me, since the syntax here has *changed*, it's not being extended.

>> OK.  It would be nice to have just a few words in this spec about the concatenation, then.  Particularly because the commas are now optional unlike in 8610.
> 
> I would like to avoid the term concatenation, as we are not concatenating data-model level strings here, but we are assembling CBOR sequences (sequences of encoded data items).  This is briefly described in Section 4.2 of RFC 8742; this doesn’t outright extend EDN, but is the basis for allowing seq at the top level of the current ABNF.

8742 is not referenced by this doc.  This doc should have a complete description of at least the semantic notions that an implementor needs to process the ABNF.

>> There's not a *lot* of overhead, but there's enough that it will become noticeable, I think.  Therefore, I'm going to interpolate the app-string grammars into my larger grammar.  I hope I do it in a way that is similar enough to everyone else that it will interoperate and still allow extensibility correctly.  Maybe I'll get it right.
> 
> I think that is the point of the discussion about keeping some of PR # 49 around, either as an alternative (e.g., in an appendix) or as some wiki.

Then let me state for the record that I agree with Rohan.  I reserve the right to find nits in PR#49 until after I've implemented it, but it looks more-or-less on the right track to me.


>>>> - (nit) I'd prefer basenumber to be split into 3 or 4 rules, one for each base, since each needs special processing.
>>> 
>>> I personally like the more compact way, but I’ll put up a change proposal in a PR.
>> 
>> It's not worth the change if others had tools available to them to process 0x 0o and 0b strings (including hex floats) all in one place.  Maybe I could have gotten all of them except hexfloats if I'd been willing to use eval().  If we are going to make a change for this, I'd suggest hexfloats be separated from hexints as well, since they are more likely to need custom code.
> 
> Done in an update to #58.
> As I wrote, my code is now trivial, if a bit more repetitive.

Nod.  Like I said, it's a nit that I have worked around by doing this separation in my own version of the grammar.  The ABNF doesn't need to change unless someone else thinks it's useful.  

I strongly agree with the other change in PR#59 though.  "nonfin" is a much better name for the rule that includes NaN.

>>> ### Document structure
>> 
>> I *didn't* read 8610 as I sat down to implement this, and didn't understand that it was important to do so from the text.  I started work on a CDDL implementation months ago, but never got very far.  Nevertheless, I expected CDDL to be completely unrelated to implementing diagnostic notation,
> 
> It is.  It just happened to a document that we were completing at the time we decided to write up what became the E in EDN.
> I’ll look for opportunities to clarify this.

I'd much rather we make this doc not depend on 8610.  This doc does not say it updates 8610, and has an incompatible syntax.  Again, I'm willing to provide text and time.

>> and assumed I didn't need that doc.  In particular, section 1 says:
>> 
>> "This document sets forth a further step of evolution of EDN, and it is intended to serve as a single reference target in specifications that use EDN."
> 
> A single reference target doesn’t imply that this needs to be without references…

Fair enough.  That doesn't mean I agree that this doc achieves its goals in its current state, however.

> The current CSV represents a failure as 
> 
> -,mybadedn,
> 
> (i.e., “-“ for not OK, and leave the third column empty).
> 
>> Those probably should be in a separate file.  
> 
> I like them together with the related succeeding cases.

Ah, now that I understand the syntax, that works for me.  What would you think about having column headers to make it more clear?

I would have had the third column be just hex-encoded CBOR, but can live with the h'' notation I guess.

— 
Joe Hildebrand