[llvm-commits] [PATCH] YAML I/O for review

[Sean Silva]

> My model is that when inputting yaml, if some string cannot be converted to a value, you get a diagnostic because it is most likely a typo in the yaml document.  On the other hand, when outputting yaml, if some runtime value cannot be converted, that means there is a bug in the program.

Ah, yes that makes sense. For some reason I was thinking that the
assertion could fire on input.

>> Could you provide a bit more info about the decision to use void* for
>> the context? What alternatives have you considered?
> I just wanted something as general as possible.  Every use of YAML I/O may need a different aspect of its context.    Is there some better model for this?

I don't have anything against it per se. Just seems a little C-style.

-- Sean Silva


On Thu, Dec 6, 2012 at 5:41 PM, Nick Kledzik <kledzik at apple.com> wrote:
> On Dec 4, 2012, at 7:18 PM, Sean Silva wrote:
>> A first round of comments from just reading the (fantastic :) documentation.
>>
>> +When writing YAML, if the value being written does not match any of the values
>> +specified by the enumCase() methods, a runtime assertion is triggered.
>>
>> Assertion? Or just diagnostic?
> assertion.
>
> My model is that when inputting yaml, if some string cannot be converted to a value, you get a diagnostic because it is most likely a typo in the yaml document.  On the other hand, when outputting yaml, if some runtime value cannot be converted, that means there is a bug in the program.
>
>
>> +The context value is just a void*.  All your traits which use the context
>> +and operate on your native data types, need to agree what the context value
>> +actually is.  It could be a pointer to an object or struct which your various
>> +traits use to shared context sensitive information.
>>
>> Could you provide a bit more info about the decision to use void* for
>> the context? What alternatives have you considered?
> I just wanted something as general as possible.  Every use of YAML I/O may need a different aspect of its context.    Is there some better model for this?
>
>
>> +On the other hand, if the top level data structure you are streaming as YAML
>> +is a sequence, Output assumes that each element of the sequence is a document.
>> +In order to generate a single document that is itself a sequence, you need
>> +to wrap the data in another sequence.
>>
>> In what situations is it beneficial to have this be the default? My
>> gut feels that outputting the sequence as a single document containing
>> the sequence is the more obvious thing to do. You wouldn't even have
>> to document it, because any programmer's instant reaction will be to
>> just "for (auto &el : seq) yout << el;", knowing that each element
>> will be output as its own document. It also seems to create the nice
>> invariant that each << creates a single document. The current default
>> forces this non-obvious wrapping hack to get the non-default behavior.
>
> Unfortunately, you cannot just append yaml documents into a file.  The token "---" is used to start a new document and the token "…" is used after the last document.  So, if YAML I/O only wrote the interior content, then clients would need to remember to output the other tokens.  And that also makes reading and writing no longer symmetrical.
>
> The patch I posted a few days ago just assumed vectors were sequences (or documents).   I'm working on a change to introduce a trait for modeling any native container data structure as a sequence.  Given this, I should also have a trait for document lists.  Then there is no longer any ambiguity.  Either what you are streaming has a sequence trait on its type or a document trait on its type.
>
> Thanks for your reviews!
>
> -Nick
>