[PATCH] YAML Input improvement

[Jonathan Anderson via llvm-commits]

Good evening,

I’ve just confirmed that the attached patch still builds and passes 
all expected tests on the head revision (both on FreeBSD and Mac OS X). 
Just wondering if someone might be willing to commit it?

Thanks,


Jonathan Anderson


On 26 Nov 2015, at 11:10, Jonathan Anderson wrote:

> Ping?
>
> On 19 Nov 2015, at 0:31, Jonathan Anderson wrote:
>
>> Good evening,
>>
>> It’s been awhile since I pinged the list with this patch, but the 
>> recent commit of r253311 has spurred me to try 
>> http://reviews.llvm.org/D11137 again.
>>
>> On 24 Jul 2015, at 18:35, Alex L wrote:
>>>> diff --git a/include/llvm/Support/YAMLTraits.h
>>> b/include/llvm/Support/YAMLTraits.h
>>>> index c04294a..6ba839c 100644
>>>> --- a/include/llvm/Support/YAMLTraits.h
>>>> +++ b/include/llvm/Support/YAMLTraits.h
>>>> @@ -976,6 +976,10 @@ public:
>>>> void *Ctxt = nullptr,
>>>> SourceMgr::DiagHandlerTy DiagHandler = nullptr,
>>>> void *DiagHandlerCtxt = nullptr);
>>>> +  Input(MemoryBuffer &Input,
>>>
>>> I think we should pass in a MemoryBufferRef here.
>>> This would make the API more consistent with the YAML stream's API.
>>
>> Fixed.
>>
>>
>>>> diff --git a/unittests/Support/YAMLIOTest.cpp
>>> b/unittests/Support/YAMLIOTest.cpp
>>>> index e7affa1..9b34109 100644
>>>> --- a/unittests/Support/YAMLIOTest.cpp
>>>> +++ b/unittests/Support/YAMLIOTest.cpp
>>>> @@ -233,6 +233,15 @@ TEST(YAMLIO, TestSequenceMapWriteAndRead) {
>>>> }
>>>> }
>>>>
>>>> +//
>>>> +// Test YAML filename handling.
>>>> +//
>>>> +TEST(YAMLIO, TestGivenFilename) {
>>>> +  // NOTE: Input doesn't expose enough information to retrieve the
>>> filename.
>>>> +  auto Buffer = llvm::MemoryBuffer::getMemBuffer("{ x: 42 }",
>>> "foo.yaml");
>>>> +  Input yin(*Buffer);
>>>> +}
>>>
>>> While we can't access the filename directly, we still would like to 
>>> test
>>> that
>>> now the yaml::Input class uses the filename from the provided buffer
>>> reference.
>>> We can do it by checking that the reported diagnostics actually use 
>>> the
>>> specified filename, like in the example below:
>>>
>>> [...]
>>>
>>> Please update the test case to perform some kind of check like this.
>>> Feel free to use the example code I posted above.
>>
>> Thanks: I used the test that you suggested.
>>
>> So, hopefully this addresses the last of your comments (which were 
>> very helpful, thanks!). Might we be able to move forward with this 
>> patch?
>>
>> Thanks,
>>
>>
>> Jonathan Anderson

--
Assistant Professor
Electrical and Computer Engineering
Memorial University of Newfoundland

http://www.engr.mun.ca/~anderson
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: D11137.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151204/378790a2/attachment.ksh>