[llvm] r204312 - Object: Don't double-escape empty hexdata

[Rui Ueyama]

I don't know if YAMLIO has changed, but I feel like handling string
quotation (choose to quote a string with '', "" or don't quote) should be
handled by YAMLIO itself and should not be a concern of a YAMLIO user, so
that we will never create malformed YAML files. In that sense this looks
correct to me.

On Mon, Mar 31, 2014 at 10:44 AM, Sean Silva <silvas at purdue.edu> wrote:

> That's really weird... I thought that the raw_ostream that this was
> outputting to was meant to go directly into the output file in the place of
> a YAML scalar (I wrote BinaryRef). Maybe something in YAMLIO changed? Where
> are the single quotes being added?
>
> -- Sean Silva
>
>
> On Mon, Mar 31, 2014 at 1:31 PM, David Majnemer <david.majnemer at gmail.com>wrote:
>
>> The escaping logic will ensure we get a pair of single quotes for
>> zero-length data.
>> Previously, we would represent an empty string as a pair of single quotes
>> around a pair of double quotes for an empty string.
>>
>> If you disagree, please construct a test-case in which the behavior post
>> r204312 is wrong.
>>
>> --
>> David Majnemer
>>
>> On Mon Mar 31 2014 at 10:26:51 AM, Sean Silva <silvas at purdue.edu> wrote:
>>
>>> ???
>>>
>>> This doesn't make any sense. The emission of the empty quotes is there
>>> on purpose (hence the test case!) and actually was put there to fix a bug
>>> where BinaryRef::writeAsHex would emit an empty string, which is not a
>>> valid YAML scalar. If you want to just get a raw hex string, I would
>>> recommend segregating the previous functionality into a method
>>> writeAsHexScalar and add a new writeAsHexRaw with the behavior that you are
>>> wanting here.
>>>
>>> Rui, were you the one that ran into the issue with the empty string and
>>> added this test case?
>>>
>>> -- Sean Silva
>>>
>>>
>>> On Thu, Mar 20, 2014 at 2:28 AM, David Majnemer <
>>> david.majnemer at gmail.com> wrote:
>>>
>>>> Author: majnemer
>>>> Date: Thu Mar 20 01:28:52 2014
>>>> New Revision: 204312
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=204312&view=rev
>>>> Log:
>>>> Object: Don't double-escape empty hexdata
>>>>
>>>> We would emit a pair of double quotes inside a pair of single quotes.
>>>> Just use a pair of single quotes.
>>>>
>>>> Modified:
>>>>     llvm/trunk/lib/Object/YAML.cpp
>>>>     llvm/trunk/unittests/Object/YAMLTest.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Object/YAML.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/YAML.cpp?rev=204312&r1=204311&r2=204312&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Object/YAML.cpp (original)
>>>> +++ llvm/trunk/lib/Object/YAML.cpp Thu Mar 20 01:28:52 2014
>>>> @@ -51,10 +51,8 @@ void BinaryRef::writeAsBinary(raw_ostrea
>>>>  }
>>>>
>>>>  void BinaryRef::writeAsHex(raw_ostream &OS) const {
>>>> -  if (binary_size() == 0) {
>>>> -    OS << "\"\"";
>>>> +  if (binary_size() == 0)
>>>>      return;
>>>> -  }
>>>>    if (DataIsHexString) {
>>>>      OS.write((const char *)Data.data(), Data.size());
>>>>      return;
>>>>
>>>> Modified: llvm/trunk/unittests/Object/YAMLTest.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Object/YAMLTest.cpp?rev=204312&r1=204311&r2=204312&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/unittests/Object/YAMLTest.cpp (original)
>>>> +++ llvm/trunk/unittests/Object/YAMLTest.cpp Thu Mar 20 01:28:52 2014
>>>> @@ -34,5 +34,5 @@ TEST(ObjectYAML, BinaryRef) {
>>>>    llvm::raw_svector_ostream OS(Buf);
>>>>    yaml::Output YOut(OS);
>>>>    YOut << BH;
>>>> -  EXPECT_NE(OS.str().find("\"\""), StringRef::npos);
>>>> +  EXPECT_NE(OS.str().find("''"), StringRef::npos);
>>>>  }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140331/e317919d/attachment.html>