[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