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

[Sean Silva]

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?

+  struct IntVector : public std::vector<int> {
+    // The existence of this member causes YAML I/O to use a flow sequence
+    static const bool flow = true;
+  };

Maybe something that is less likely than just "flow". Maybe
`YAMLFormatAsFlow`? It also will make it self-documenting.

+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?

+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.

-- Sean Silva

On Tue, Dec 4, 2012 at 4:25 PM, Nick Kledzik <kledzik at apple.com> wrote:
> Attached is a patch to add functionality to llvm to aid in the conversion between in-memory data structures and YAML text files.
>
>
>
>
> The patch includes the implementation, test cases, and documentation.
>
> I've included a PDF of the documentation, so you don't have to install the patch and run sphinx to read it.
>
>
>
>
>
> Back in August I proposed a different implementation that required all in-memory data structure to derive from some yaml classes I had defined.  It was suggested that using a traits based approach could eliminate that intrusiveness and allow YAML I/O to work with any existing data structures by defining template specializations on the existing data types.   Doing that was straight forward with C++11, but getting it to work with older C++ compilers was challenging.  I finally found an alternate way to do the trait testing that should work with all C++ compilers.
>
> -Nick
>
>
>
>
>