Add filename support to yaml::Input.

[trombonehero]

Summary

The current yaml::Input constructor only takes a StringRef for its first
parameter: clients pass in raw string data. This means that, for the
typical use case of opening a file and parsing it, we throw away the
filename information and report errors as occurring in "YAML".

This commit adds an alterate yaml::Input constructor that takes a reference
to a MemoryBuffer, then extracts the buffer and whatever identifier is
associated with that buffer and passes them to the underlying yaml::Stream
as a MemoryBufferRef.

[arphaman]

Hi Jonathan,

This patch lacks an appropriate test, please add a test case for the new constructor in the unit test file for yaml traits ('unittest/Support/YAMLIOTest.cpp').

A minor style comment is included below:

include/llvm/Support/YAMLTraits.h

979

The ampersand should be together with the name of the variable.
This should be done for the constructor in the CPP file as well.

[trombonehero]

• Add unit test for new yaml::Input constructor.

[trombonehero]

• Put reference ampersands with variable name.

[trombonehero]

Adding @bkramer and @kledzik as reviewers, since they seem to have written the majority of YAMLTraits.cpp.

[trombonehero]

• Let yaml::Input take a MemoryBufferRef.

[trombonehero]

• Make use of new MemoryBufferRef constructor from r253311.

[trombonehero]

@emaste: ping?

[arphaman]

Please adjust the new constructor as requested, otherwise LGTM. Sorry that it took so long to get back.

lib/Support/YAMLTraits.cpp

63

Nitpick: on ToT the Input::Input above is formatted differently. It would be more appropriate to format the new constructor declaration similarly, something like should work:

Input::Input(MemoryBufferRef Input, void *Ctxt,
             SourceMgr::DiagHandlerTy DiagHandler, void *DiagHandlerCtxt)
    : IO(Ctxt), Strm(new Stream(Input, SrcMgr, false, &EC)),
      CurrentNode(nullptr) {

64

Please pass in &EC to the Stream constructor as the other Input constructor passes it into Stream on ToT.

[arphaman]

Jonathan, I have noticed that llvm-commits wasn't subscribed to this patch after I accepted it. I have to withdraw my LGTM, since patches to LLVM should go through llvm-commits according to the guidelines [1]. Can you please re-post this patch with llvm-commits added as a subscriber and me as a reviewer? Thank you for your understanding!

[1] http://llvm.org/docs/Phabricator.html

include/llvm/Support/YAMLTraits.h

 public:
   // Construct a yaml Input object from a StringRef and optional
   // user-data. The DiagHandler can be specified to provide
   // alternative error reporting.
   Input(StringRef InputContent,
         void *Ctxt = nullptr,
         SourceMgr::DiagHandlerTy DiagHandler = nullptr,
         void *DiagHandlerCtxt = nullptr);
+  Input(MemoryBuffer& Input,
+        void *Ctxt = nullptr,
+        SourceMgr::DiagHandlerTy DiagHandler = nullptr,
+        void *DiagHandlerCtxt = nullptr);
   ~Input() override;
 
   // Check if there was an syntax or semantic error during parsing.
   std::error_code error();
 
 private:
   bool outputting() override;
   bool mapTag(StringRef, bool) override;

lib/Support/YAMLTraits.cpp

Input::Input(StringRef InputContent,
   : IO(Ctxt),
     Strm(new Stream(InputContent, SrcMgr)),
     CurrentNode(nullptr) {
   if (DiagHandler)
     SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt);
   DocIterator = Strm->begin();
 }
 
+Input::Input(MemoryBuffer& Input,
+             void *Ctxt,
+             SourceMgr::DiagHandlerTy DiagHandler,
+             void *DiagHandlerCtxt)
+  : IO(Ctxt),
+    Strm(new Stream(
+      MemoryBufferRef(Input.getBuffer(), Input.getBufferIdentifier()), SrcMgr)),
+    CurrentNode(nullptr) {
+  if (DiagHandler)
+    SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt);
+  DocIterator = Strm->begin();
+}
+
 Input::~Input() {
 }
 
 std::error_code Input::error() { return EC; }
 
 // Pin the vtables to this file.
 void Input::HNode::anchor() {}
 void Input::EmptyHNode::anchor() {}