[llvm] r188508 - Tighten up the yamilizer so it stops eliding empty sequences if the embedded empty sequence is the first key/value in a map which is itself in a sequence.

[Aaron Ballman]

Author: aaronballman
Date: Thu Aug 15 18:17:53 2013
New Revision: 188508

URL: http://llvm.org/viewvc/llvm-project?rev=188508&view=rev
Log:
Tighten up the yamilizer so it stops eliding empty sequences if the embedded empty sequence is the first key/value in a map which is itself in a sequence.

Patch with help from Nick Kledzik.

Modified:
    llvm/trunk/include/llvm/Support/YAMLTraits.h
    llvm/trunk/lib/Support/YAMLTraits.cpp
    llvm/trunk/unittests/Support/YAMLIOTest.cpp

Modified: llvm/trunk/include/llvm/Support/YAMLTraits.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/YAMLTraits.h?rev=188508&r1=188507&r2=188508&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/YAMLTraits.h (original)
+++ llvm/trunk/include/llvm/Support/YAMLTraits.h Thu Aug 15 18:17:53 2013
@@ -323,6 +323,7 @@ public:
   virtual bool preflightElement(unsigned, void *&) = 0;
   virtual void postflightElement(void*) = 0;
   virtual void endSequence() = 0;
+  virtual bool canElideEmptySequence() = 0;
 
   virtual unsigned beginFlowSequence() = 0;
   virtual bool preflightFlowElement(unsigned, void *&) = 0;
@@ -388,7 +389,7 @@ public:
   typename llvm::enable_if_c<has_SequenceTraits<T>::value,void>::type
   mapOptional(const char* Key, T& Val) {
     // omit key/value instead of outputting empty sequence
-    if ( this->outputting() && !(Val.begin() != Val.end()) )
+    if ( this->canElideEmptySequence() && !(Val.begin() != Val.end()) )
       return;
     this->processKey(Key, Val, false);
   }
@@ -715,6 +716,7 @@ private:
   virtual void endBitSetScalar();
   virtual void scalarString(StringRef &);
   virtual void setError(const Twine &message);
+  virtual bool canElideEmptySequence();
 
   class HNode {
   public:
@@ -837,7 +839,7 @@ public:
   virtual void endBitSetScalar();
   virtual void scalarString(StringRef &);
   virtual void setError(const Twine &message);
-
+  virtual bool canElideEmptySequence();
 public:
   // These are only used by operator<<. They could be private
   // if that templated operator could be made a friend.

Modified: llvm/trunk/lib/Support/YAMLTraits.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLTraits.cpp?rev=188508&r1=188507&r2=188508&view=diff
==============================================================================
--- llvm/trunk/lib/Support/YAMLTraits.cpp (original)
+++ llvm/trunk/lib/Support/YAMLTraits.cpp Thu Aug 15 18:17:53 2013
@@ -334,6 +334,10 @@ void Input::setError(const Twine &Messag
   this->setError(CurrentNode, Message);
 }
 
+bool Input::canElideEmptySequence() {
+  return false;
+}
+
 Input::MapHNode::~MapHNode() {
   for (MapHNode::NameToNode::iterator i = Mapping.begin(), End = Mapping.end();
                                                                 i != End; ++i) {
@@ -532,6 +536,19 @@ void Output::scalarString(StringRef &S)
 void Output::setError(const Twine &message) {
 }
 
+bool Output::canElideEmptySequence() {
+  // Normally, with an optional key/value where the value is an empty sequence,
+  // the whole key/value can be not written.  But, that produces wrong yaml
+  // if the key/value is the only thing in the map and the map is used in
+  // a sequence.  This detects if the this sequence is the first key/value
+  // in map that itself is embedded in a sequnce.
+  if (StateStack.size() < 2) 
+    return true;
+  if (StateStack.back() != inMapFirstKey) 
+    return true;
+  return (StateStack[StateStack.size()-2] != inSeq);
+}
+
 void Output::output(StringRef s) {
   Column += s.size();
   Out << s;

Modified: llvm/trunk/unittests/Support/YAMLIOTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/YAMLIOTest.cpp?rev=188508&r1=188507&r2=188508&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/YAMLIOTest.cpp (original)
+++ llvm/trunk/unittests/Support/YAMLIOTest.cpp Thu Aug 15 18:17:53 2013
@@ -1297,3 +1297,66 @@ TEST(YAMLIO, TestReadBuiltInTypesHex64Er
   EXPECT_TRUE(yin.error());
 }
 
+struct OptionalTest {
+  std::vector<int> Numbers;
+};
+
+struct OptionalTestSeq {
+  std::vector<OptionalTest> Tests;
+};
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(OptionalTest);
+namespace llvm {
+namespace yaml {
+  template <>
+  struct MappingTraits<OptionalTest> {
+    static void mapping(IO& IO, OptionalTest &OT) {
+      IO.mapOptional("Numbers", OT.Numbers);
+    }
+  };
+
+  template <>
+  struct MappingTraits<OptionalTestSeq> {
+    static void mapping(IO &IO, OptionalTestSeq &OTS) {
+      IO.mapOptional("Tests", OTS.Tests);
+    }
+  };
+}
+}
+
+TEST(YAMLIO, SequenceElideTest) {
+  // Test that writing out a purely optional structure with its fields set to
+  // default followed by other data is properly read back in.
+  OptionalTestSeq Seq;
+  OptionalTest One, Two, Three, Four;
+  int N[] = {1, 2, 3};
+  Three.Numbers.assign(N, N + 3);
+  Seq.Tests.push_back(One);
+  Seq.Tests.push_back(Two);
+  Seq.Tests.push_back(Three);
+  Seq.Tests.push_back(Four);
+
+  std::string intermediate;
+  {
+    llvm::raw_string_ostream ostr(intermediate);
+    Output yout(ostr);
+    yout << Seq;
+  }
+
+  Input yin(intermediate);
+  OptionalTestSeq Seq2;
+  yin >> Seq2;
+  
+  EXPECT_FALSE(yin.error());
+
+  EXPECT_EQ(4UL, Seq2.Tests.size());
+
+  EXPECT_TRUE(Seq2.Tests[0].Numbers.empty());
+  EXPECT_TRUE(Seq2.Tests[1].Numbers.empty());
+
+  EXPECT_EQ(1, Seq2.Tests[2].Numbers[0]);
+  EXPECT_EQ(2, Seq2.Tests[2].Numbers[1]);
+  EXPECT_EQ(3, Seq2.Tests[2].Numbers[2]);
+
+  EXPECT_TRUE(Seq2.Tests[3].Numbers.empty());
+}