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

Alexander Kornienko alexfh at google.com
Fri Nov 15 12:18:26 PST 2013

  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:

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:

(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 @@
-  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.


More information about the llvm-commits mailing list