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

[Nick Kledzik]

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