[PATCH] Fix YAML sequences with default entries

Nick Kledzik kledzik at apple.com
Tue Aug 13 18:50:14 PDT 2013


Aaron,

Thanks for digging into this and making a patch and test case.  After reviewing it, I think the change is too strong and prevents some valid key/value-seq elides.  For instance, if the map has another key/value pair, it is ok to elide the empty sequence because the other key/value keeps a entry in the top level sequence . 

Here is a patch that tightens it up a bit and only stops eliding empty sequences if the embedded empty sequence is the first key/value in a map which is itself in a sequence. 

I also, moved the implementation of the checks out of the header and into YAMLTraits.cpp, so that all clients don’t carry a copy.  And I changed:
  EXPECT_EQ(2, Seq2.Tests.size());
to:
  EXPECT_EQ(2UL, Seq2.Tests.size()); 
to prevent compiler comparison warnings.

Please try this out on whatever real world yaml case you were using to verify it fixes your problem.  Thanks.

-Nick


On Aug 13, 2013, at 4:09 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> When creating a YAML sequence where an element is a mapping that is
> defaulted, the current implementation elides the element entirely,
> which causes round-tripping problems.  Since the output drops the
> defaulted entries, the input never receives the correct number of
> entries in the sequence.
> 
> The attached patch resolves this issue by checking to see whether a
> mapping operation is currently processing a sequence before deciding
> to elide the mapped element.  Unit tests demonstrating the issue are
> included.
> 
> ~Aaron
> <yaml.patch>





More information about the llvm-commits mailing list