Bug 30934 - YAML error not reported by ``yaml::Input::error()``

[serge-sans-paille]

@mehdi_amini up :-)

[mehdi_amini]

lib/Support/YAMLTraits.cpp

115 ↗

(On Diff #77249)

So it seems that any error checking on Input should also check on the Stream itself. It may be less error prone to refactor this check into a single method and turn all the tests into if (hasError()) return;

[serge-sans-paille]

Refactoring done!

[serge-sans-paille]

lib/Support/YAMLTraits.cpp

115 ↗

(On Diff #77249)

That's my feeling too. I did the proposed refactoring, putting the method as private to prevent semantic conflict with the public error() method.

[mehdi_amini]

Thanks, LGTM.

[mehdi_amini]

Serge: the unittest is not passing, can you have a look?

[serge-sans-paille]

This patch seemsmore robust: when a subcomponent reports an error, it may set the parent error_code.

[serge-sans-paille]

Different system, different header deps; I made this one explicit.

llvm/trunk/include/llvm/Support/YAMLParser.h

 
 #ifndef LLVM_SUPPORT_YAMLPARSER_H
 #define LLVM_SUPPORT_YAMLPARSER_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/SMLoc.h"
 #include <map>
+#include <system_error>
 #include <utility>
 
 namespace llvm {
 class MemoryBufferRef;
 class SourceMgr;
 class Twine;
 class raw_ostream;
 

 /// \brief Escape \a Input for a double quoted scalar.
 std::string escape(StringRef Input);
 
 /// \brief This class represents a YAML stream potentially containing multiple
 ///        documents.
 class Stream {
 public:
   /// \brief This keeps a reference to the string referenced by \p Input.
-  Stream(StringRef Input, SourceMgr &, bool ShowColors = true);
+  Stream(StringRef Input, SourceMgr &, bool ShowColors = true,
+         std::error_code *EC = nullptr);
 
-  Stream(MemoryBufferRef InputBuffer, SourceMgr &, bool ShowColors = true);
+  Stream(MemoryBufferRef InputBuffer, SourceMgr &, bool ShowColors = true,
+         std::error_code *EC = nullptr);
   ~Stream();
 
   document_iterator begin();
   document_iterator end();
   void skip();
   bool failed();
   bool validate() {
     skip();

llvm/trunk/lib/Support/YAMLParser.cpp

static UTF8Decoded decodeUTF8(StringRef Range) {
   return std::make_pair(0, 0);
 }
 
 namespace llvm {
 namespace yaml {
 /// @brief Scans YAML tokens from a MemoryBuffer.
 class Scanner {
 public:
-  Scanner(StringRef Input, SourceMgr &SM, bool ShowColors = true);
-  Scanner(MemoryBufferRef Buffer, SourceMgr &SM_, bool ShowColors = true);
+  Scanner(StringRef Input, SourceMgr &SM, bool ShowColors = true,
+          std::error_code *EC = nullptr);
+  Scanner(MemoryBufferRef Buffer, SourceMgr &SM_, bool ShowColors = true,
+          std::error_code *EC = nullptr);
 
   /// @brief Parse the next token and return it without popping it.
   Token &peekNext();
 
   /// @brief Parse the next token and pop it from the queue.
   Token getNext();
 
   void printError(SMLoc Loc, SourceMgr::DiagKind Kind, const Twine &Message,
                   ArrayRef<SMRange> Ranges = None) {
     SM.PrintMessage(Loc, Kind, Message, Ranges, /* FixIts= */ None, ShowColors);
   }
 
   void setError(const Twine &Message, StringRef::iterator Position) {
     if (Current >= End)
       Current = End - 1;
 
+    // propagate the error if possible
+    if (EC)
+      *EC = make_error_code(std::errc::invalid_argument);
+
     // Don't print out more errors after the first one we encounter. The rest
     // are just the result of the first, and have no meaning.
     if (!Failed)
       printError(SMLoc::getFromPointer(Current), SourceMgr::DK_Error, Message);
     Failed = true;
   }
 
   void setError(const Twine &Message) {
private:
   ///        can produce multiple tokens (e.g. BlockEnd).
   TokenQueueT TokenQueue;
 
   /// @brief Indentation levels.
   SmallVector<int, 4> Indents;
 
   /// @brief Potential simple keys.
   SmallVector<SimpleKey, 4> SimpleKeys;
+
+  std::error_code *EC;
 };
 
 } // end namespace yaml
 } // end namespace llvm
 
 /// encodeUTF8 - Encode \a UnicodeScalarValue in UTF-8 and append it to result.
 static void encodeUTF8( uint32_t UnicodeScalarValue
                       , SmallVectorImpl<char> &Result) {
else if ((unsigned char)*i < 0x20) { // Control characters not handled above.
       }
       i += UnicodeScalarValue.second - 1;
     } else
       EscapedInput.push_back(*i);
   }
   return EscapedInput;
 }
 
-Scanner::Scanner(StringRef Input, SourceMgr &sm, bool ShowColors)
-    : SM(sm), ShowColors(ShowColors) {
+Scanner::Scanner(StringRef Input, SourceMgr &sm, bool ShowColors,
+                 std::error_code *EC)
+    : SM(sm), ShowColors(ShowColors), EC(EC) {
   init(MemoryBufferRef(Input, "YAML"));
 }
 
-Scanner::Scanner(MemoryBufferRef Buffer, SourceMgr &SM_, bool ShowColors)
-    : SM(SM_), ShowColors(ShowColors) {
+Scanner::Scanner(MemoryBufferRef Buffer, SourceMgr &SM_, bool ShowColors,
+                 std::error_code *EC)
+    : SM(SM_), ShowColors(ShowColors), EC(EC) {
   init(Buffer);
 }
 
 void Scanner::init(MemoryBufferRef Buffer) {
   InputBuffer = Buffer;
   Current = InputBuffer.getBufferStart();
   End = InputBuffer.getBufferEnd();
   Indent = -1;
if (!(isBlankOrBreak(Current)
                       && *(Current + 1) == ':'
                       && !isBlankOrBreak(Current + 2)))
     return scanPlainScalar();
 
   setError("Unrecognized character while tokenizing.");
   return false;
 }
 
-Stream::Stream(StringRef Input, SourceMgr &SM, bool ShowColors)
-    : scanner(new Scanner(Input, SM, ShowColors)), CurrentDoc() {}
-
-Stream::Stream(MemoryBufferRef InputBuffer, SourceMgr &SM, bool ShowColors)
-    : scanner(new Scanner(InputBuffer, SM, ShowColors)), CurrentDoc() {}
+Stream::Stream(StringRef Input, SourceMgr &SM, bool ShowColors,
+               std::error_code *EC)
+    : scanner(new Scanner(Input, SM, ShowColors, EC)), CurrentDoc() {}
+
+Stream::Stream(MemoryBufferRef InputBuffer, SourceMgr &SM, bool ShowColors,
+               std::error_code *EC)
+    : scanner(new Scanner(InputBuffer, SM, ShowColors, EC)), CurrentDoc() {}
 
 Stream::~Stream() {}
 
 bool Stream::failed() { return scanner->failed(); }
 
 void Stream::printError(Node *N, const Twine &Msg) {
   scanner->printError( N->getSourceRange().Start
                      , SourceMgr::DK_Error

llvm/trunk/lib/Support/YAMLTraits.cpp

 void IO::setContext(void *Context) {
   Ctxt = Context;
 }
 
 //===----------------------------------------------------------------------===//
 //  Input
 //===----------------------------------------------------------------------===//
 
-Input::Input(StringRef InputContent,
-             void *Ctxt,
-             SourceMgr::DiagHandlerTy DiagHandler,
-             void *DiagHandlerCtxt)
-  : IO(Ctxt),
-    Strm(new Stream(InputContent, SrcMgr)),
+Input::Input(StringRef InputContent, void *Ctxt,
+             SourceMgr::DiagHandlerTy DiagHandler, void *DiagHandlerCtxt)
+    : IO(Ctxt), Strm(new Stream(InputContent, SrcMgr, false, &EC)),
       CurrentNode(nullptr) {
   if (DiagHandler)
     SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt);
   DocIterator = Strm->begin();
 }
 
 Input::~Input() {
 }
 

llvm/trunk/unittests/Support/YAMLIOTest.cpp

EXPECT_EQ("---\n"
             "Simple:          \n"
             "  B:               2\n"
             "  C:               3\n"
             "  Context:         2\n"
             "...\n",
             out);
   out.clear();
 }
+
+TEST(YAMLIO, InvalidInput) {
+  // polluting 1 value in the sequence
+  Input yin("---\n- foo:  3\n  bar:  5\n1\n- foo:  3\n  bar:  5\n...\n");
+  std::vector<FooBar> Data;
+  yin >> Data;
+  EXPECT_TRUE((bool)yin.error());
+}