[PATCH] Fix YAML sequences with default entries

Nick Kledzik kledzik at apple.com
Wed Aug 14 12:42:15 PDT 2013


[resend with patch attached]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: yaml.patch
Type: application/octet-stream
Size: 4390 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130814/7deacfe7/attachment.obj>
-------------- next part --------------

On Aug 13, 2013, at 6:50 PM, Nick Kledzik <kledzik at apple.com> wrote:
> 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