[PATCH] [YAML] Recover gracefully when deserializing invalid YAML input.

[Alexander Kornienko]

  Thanks for the updated patch! It looks good overall. Please see a few comments inline.


================
Comment at: unittests/Support/YAMLIOTest.cpp:1372
@@ +1371,3 @@
+    // message generated in the constructor of Input.
+    Input yin("{foo:3, bar: 5}", NULL /* ctxt */, suppressErrorMessages);
+    yin >> doc;
----------------
nit: in LLVM it's more common to write:
/*Ctxt=*/NULL,

================
Comment at: include/llvm/Support/YAMLTraits.h:692
@@ +691,3 @@
+  Input(StringRef InputContent,
+        void *Ctxt=NULL,
+        SourceMgr::DiagHandlerTy Handler=NULL,
----------------
nit: Please add spaces around assignments.

I'd also rename Handler to DiagHandler and rephrase the comment:
"The DiagHandler can be specified to provide alternative error reporting."

================
Comment at: include/llvm/Support/YAMLTraits.h:702
@@ -695,2 +701,3 @@
+  // Overrides the any handler set in the constructor of Input.
   void setDiagHandler(llvm::SourceMgr::DiagHandlerTy Handler, void *Ctxt = 0);
 
----------------
I don't see any need in this method, as it's only used directly after constructor (which can be wrong, if the constructor call generates errors on its own). So I suggest removing it.

I only could find usages of this method in two files:
unittests/Support/YAMLIOTest.cpp
tools/clang/tools/extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp

(the second one resides in a different branch of the repository, and can be updated separately after this patch gets in).

================
Comment at: lib/Support/YAMLTraits.cpp:118
@@ +117,3 @@
+  // when to fail until we reach this point.
+  if (!CurrentNode) {
+    if (Required)
----------------
Is CurrentNode NULL only when the document is empty? Can it be NULL when parsing invalid documents?

================
Comment at: lib/Support/YAMLTraits.cpp:150
@@ -124,2 +149,3 @@
     return;
-  MapHNode *MN = dyn_cast<MapHNode>(CurrentNode);
+  // CurrentNode can be null if the document is empty
+  MapHNode *MN = dyn_cast_or_null<MapHNode>(CurrentNode);
----------------
nit: please put a period in the end of the comment.


http://llvm-reviews.chandlerc.com/D1236