Index: include/llvm/Support/YAMLTraits.h =================================================================== --- include/llvm/Support/YAMLTraits.h +++ include/llvm/Support/YAMLTraits.h @@ -684,16 +684,18 @@ /// class Input : public IO { public: - // Construct a yaml Input object from a StringRef and optional user-data. - Input(StringRef InputContent, void *Ctxt=NULL); + // 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 = NULL, + SourceMgr::DiagHandlerTy DiagHandler = NULL, + void *DiagHandlerCtxt = NULL); ~Input(); // Check if there was an syntax or semantic error during parsing. llvm::error_code error(); - // To set alternate error reporting. - void setDiagHandler(llvm::SourceMgr::DiagHandlerTy Handler, void *Ctxt = 0); - private: virtual bool outputting(); virtual void beginMapping(); @@ -961,8 +963,8 @@ inline typename llvm::enable_if_c::value,Input &>::type operator>>(Input &yin, T &docSeq) { - yin.setCurrentDocument(); - yamlize(yin, docSeq, true); + if (yin.setCurrentDocument()) + yamlize(yin, docSeq, true); return yin; } Index: lib/Support/YAMLTraits.cpp =================================================================== --- lib/Support/YAMLTraits.cpp +++ lib/Support/YAMLTraits.cpp @@ -41,10 +41,15 @@ // Input //===----------------------------------------------------------------------===// -Input::Input(StringRef InputContent, void *Ctxt) +Input::Input(StringRef InputContent, + void *Ctxt, + SourceMgr::DiagHandlerTy DiagHandler, + void *DiagHandlerCtxt) : IO(Ctxt), Strm(new Stream(InputContent, SrcMgr)), CurrentNode(NULL) { + if (DiagHandler) + SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt); DocIterator = Strm->begin(); } @@ -55,10 +60,6 @@ return EC; } -void Input::setDiagHandler(SourceMgr::DiagHandlerTy Handler, void *Ctxt) { - SrcMgr.setDiagHandler(Handler, Ctxt); -} - bool Input::outputting() { return false; } @@ -66,6 +67,12 @@ bool Input::setCurrentDocument() { if (DocIterator != Strm->end()) { Node *N = DocIterator->getRoot(); + if (!N) { + assert(Strm->failed() && "Root is NULL iff parsing failed"); + EC = make_error_code(errc::invalid_argument); + return false; + } + if (isa(N)) { // Empty files are allowed and ignored ++DocIterator; @@ -85,7 +92,8 @@ void Input::beginMapping() { if (EC) return; - MapHNode *MN = dyn_cast(CurrentNode); + // CurrentNode can be null if the document is empty. + MapHNode *MN = dyn_cast_or_null(CurrentNode); if (MN) { MN->ValidKeys.clear(); } @@ -96,6 +104,19 @@ UseDefault = false; if (EC) return false; + + // If CurrentNode is null (for example, the document is an empty + // string), then we check if the key is required, and if so, set the + // error code. This somewhat convoluted logic is due to the fact + // that mappings without required keys can be validly deserialized + // from an empty YAML document, and so we must defer the decision of + // when to fail until we reach this point. + if (!CurrentNode) { + if (Required) + EC = make_error_code(errc::invalid_argument); + return false; + } + MapHNode *MN = dyn_cast(CurrentNode); if (!MN) { setError(CurrentNode, "not a mapping"); @@ -122,7 +143,8 @@ void Input::endMapping() { if (EC) return; - MapHNode *MN = dyn_cast(CurrentNode); + // CurrentNode can be null if the document is empty + MapHNode *MN = dyn_cast_or_null(CurrentNode); if (!MN) return; for (MapHNode::NameToNode::iterator i = MN->Mapping.begin(), @@ -263,6 +285,7 @@ } void Input::setError(HNode *hnode, const Twine &message) { + assert(hnode && "HNode must not be NULL"); this->setError(hnode->_node, message); } Index: unittests/Support/YAMLIOTest.cpp =================================================================== --- unittests/Support/YAMLIOTest.cpp +++ unittests/Support/YAMLIOTest.cpp @@ -58,15 +58,25 @@ // TEST(YAMLIO, TestMapRead) { FooBar doc; - Input yin("---\nfoo: 3\nbar: 5\n...\n"); - yin >> doc; + { + Input yin("---\nfoo: 3\nbar: 5\n...\n"); + yin >> doc; - EXPECT_FALSE(yin.error()); - EXPECT_EQ(doc.foo, 3); - EXPECT_EQ(doc.bar,5); + EXPECT_FALSE(yin.error()); + EXPECT_EQ(doc.foo, 3); + EXPECT_EQ(doc.bar, 5); + } + + { + Input yin("{foo: 3, bar: 5}"); + yin >> doc; + + EXPECT_FALSE(yin.error()); + EXPECT_EQ(doc.foo, 3); + EXPECT_EQ(doc.bar, 5); + } } - // // Test the reading of a yaml sequence of mappings // @@ -1009,8 +1019,9 @@ "c1: blue\n" "c2: purple\n" "c3: green\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> map; EXPECT_TRUE(yin.error()); } @@ -1025,8 +1036,9 @@ "f1: [ big ]\n" "f2: [ round, hollow ]\n" "f3: []\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> map; EXPECT_TRUE(yin.error()); @@ -1043,8 +1055,9 @@ "- 255\n" "- 0\n" "- 257\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1061,8 +1074,9 @@ "- 65535\n" "- 0\n" "- 66000\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1079,8 +1093,9 @@ "- 4000000000\n" "- 0\n" "- 5000000000\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1097,8 +1112,9 @@ "- 18446744073709551615\n" "- 0\n" "- 19446744073709551615\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1116,8 +1132,9 @@ "- 0\n" "- 127\n" "- 128\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1133,8 +1150,9 @@ "- 0\n" "- 127\n" "- -129\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1152,8 +1170,9 @@ "- 0\n" "- -32768\n" "- -32769\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1170,8 +1189,9 @@ "- 0\n" "- -32768\n" "- 32768\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1189,8 +1209,9 @@ "- 0\n" "- -2147483648\n" "- -2147483649\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1206,8 +1227,9 @@ "- 0\n" "- -2147483648\n" "- 2147483649\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1225,8 +1247,9 @@ "- 0\n" "- 9223372036854775807\n" "- -9223372036854775809\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1242,8 +1265,9 @@ "- 0\n" "- 9223372036854775807\n" "- 9223372036854775809\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1260,8 +1284,9 @@ "- 1000.1\n" "- -123.456\n" "- 1.2.3\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1278,8 +1303,9 @@ "- 1000.1\n" "- -123.456\n" "- 1.2.3\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1295,8 +1321,9 @@ "- 0x12\n" "- 0xFE\n" "- 0x123\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1313,8 +1340,9 @@ "- 0x0012\n" "- 0xFEFF\n" "- 0x12345\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1330,8 +1358,9 @@ "- 0x0012\n" "- 0xFEFF0000\n" "- 0x1234556789\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); @@ -1347,13 +1376,31 @@ "- 0x0012\n" "- 0xFFEEDDCCBBAA9988\n" "- 0x12345567890ABCDEF0\n" - "...\n"); - yin.setDiagHandler(suppressErrorMessages); + "...\n", + /*Ctxt=*/NULL, + suppressErrorMessages); yin >> seq; EXPECT_TRUE(yin.error()); } +TEST(YAMLIO, TestMalformedMapFailsGracefully) { + FooBar doc; + { + // We pass the suppressErrorMessages handler to handle the error + // message generated in the constructor of Input. + Input yin("{foo:3, bar: 5}", /*Ctxt=*/NULL, suppressErrorMessages); + yin >> doc; + EXPECT_TRUE(yin.error()); + } + + { + Input yin("---\nfoo:3\nbar: 5\n...\n", /*Ctxt=*/NULL, suppressErrorMessages); + yin >> doc; + EXPECT_TRUE(yin.error()); + } +} + struct OptionalTest { std::vector Numbers; }; @@ -1417,3 +1464,26 @@ EXPECT_TRUE(Seq2.Tests[3].Numbers.empty()); } + +TEST(YAMLIO, TestEmptyStringFailsForMapWithRequiredFields) { + FooBar doc; + Input yin(""); + yin >> doc; + EXPECT_TRUE(yin.error()); +} + +TEST(YAMLIO, TestEmptyStringSucceedsForMapWithOptionalFields) { + OptionalTest doc; + Input yin(""); + yin >> doc; + EXPECT_FALSE(yin.error()); +} + +TEST(YAMLIO, TestEmptyStringSucceedsForSequence) { + std::vector seq; + Input yin("", /*Ctxt=*/NULL, suppressErrorMessages); + yin >> seq; + + EXPECT_FALSE(yin.error()); + EXPECT_TRUE(seq.empty()); +}