[llvm] 3920972 - [YAML] Fix incorrect dash output in nested sequences (#116488)

[via llvm-commits]

Author: NAKAMURA Takumi
Date: 2024-11-30T22:10:31+09:00
New Revision: 39209724e66a20080bdebc609a051dfa0eb6b49f

URL: https://github.com/llvm/llvm-project/commit/39209724e66a20080bdebc609a051dfa0eb6b49f
DIFF: https://github.com/llvm/llvm-project/commit/39209724e66a20080bdebc609a051dfa0eb6b49f.diff

LOG: [YAML] Fix incorrect dash output in nested sequences (#116488)

Nested sequences could be defined but the YAML output was incorrect.
`Output::newLineCheck()` was not able to emit multiple dashes `- ` and
YAML parser sometimes didn't accept its output as the result.

This fixes for emitting corresponding dashes for consecutive
`inSeqFirstElement`, but suppresses emission to the top
`inSeqFirstElement`.

This also fixes for emitting flow elements onto nested sequences.

Added: 
    

Modified: 
    llvm/lib/Support/YAMLTraits.cpp
    llvm/unittests/Support/YAMLIOTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp
index d259da65c5cf7f..f326422138488c 100644
--- a/llvm/lib/Support/YAMLTraits.cpp
+++ b/llvm/lib/Support/YAMLTraits.cpp
@@ -837,26 +837,40 @@ void Output::newLineCheck(bool EmptySequence) {
     return;
 
   unsigned Indent = StateStack.size() - 1;
-  bool OutputDash = false;
-
-  if (StateStack.back() == inSeqFirstElement ||
-      StateStack.back() == inSeqOtherElement) {
-    OutputDash = true;
-  } else if ((StateStack.size() > 1) &&
-             ((StateStack.back() == inMapFirstKey) ||
-              inFlowSeqAnyElement(StateStack.back()) ||
-              (StateStack.back() == inFlowMapFirstKey)) &&
-             inSeqAnyElement(StateStack[StateStack.size() - 2])) {
-    --Indent;
-    OutputDash = true;
+  bool PossiblyNestedSeq = false;
+  auto I = StateStack.rbegin(), E = StateStack.rend();
+
+  if (inSeqAnyElement(*I)) {
+    PossiblyNestedSeq = true; // Not possibly but always.
+    ++Indent;
+  } else if (*I == inMapFirstKey || *I == inFlowMapFirstKey ||
+             inFlowSeqAnyElement(*I)) {
+    PossiblyNestedSeq = true;
+    ++I; // Skip back().
   }
 
-  for (unsigned i = 0; i < Indent; ++i) {
-    output("  ");
+  unsigned OutputDashCount = 0;
+  if (PossiblyNestedSeq) {
+    // Count up consecutive inSeqFirstElement from the end, unless
+    // inSeqFirstElement is the top of nested sequence.
+    while (I != E) {
+      // Don't count the top of nested sequence.
+      if (!inSeqAnyElement(*I))
+        break;
+
+      ++OutputDashCount;
+
+      // Stop counting if consecutive inSeqFirstElement ends.
+      if (*I++ != inSeqFirstElement)
+        break;
+    }
   }
-  if (OutputDash) {
+
+  for (unsigned I = OutputDashCount; I < Indent; ++I)
+    output("  ");
+
+  for (unsigned I = 0; I < OutputDashCount; ++I)
     output("- ");
-  }
 }
 
 void Output::paddedKey(StringRef key) {

diff  --git a/llvm/unittests/Support/YAMLIOTest.cpp b/llvm/unittests/Support/YAMLIOTest.cpp
index a7d1b338719f3b..c0e9c57a77f193 100644
--- a/llvm/unittests/Support/YAMLIOTest.cpp
+++ b/llvm/unittests/Support/YAMLIOTest.cpp
@@ -1537,6 +1537,93 @@ TEST(YAMLIO, TestReadWriteMySecondsSequence) {
   }
 }
 
+//===----------------------------------------------------------------------===//
+//  Test nested sequence
+//===----------------------------------------------------------------------===//
+using NestedStringSeq1 = llvm::SmallVector<std::string, 2>;
+using NestedStringSeq2 = std::array<NestedStringSeq1, 2>;
+using NestedStringSeq3 = std::vector<NestedStringSeq2>;
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(NestedStringSeq1)
+LLVM_YAML_IS_SEQUENCE_VECTOR(NestedStringSeq2)
+
+struct MappedStringSeq3 {
+  NestedStringSeq3 Seq3;
+};
+
+template <> struct llvm::yaml::MappingTraits<MappedStringSeq3> {
+  static void mapping(IO &io, MappedStringSeq3 &seq) {
+    io.mapRequired("Seq3", seq.Seq3);
+  }
+};
+
+using NestedIntSeq1 = std::array<int, 2>;
+using NestedIntSeq2 = std::array<NestedIntSeq1, 2>;
+using NestedIntSeq3 = std::array<NestedIntSeq2, 2>;
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(NestedIntSeq1)
+LLVM_YAML_IS_SEQUENCE_VECTOR(NestedIntSeq2)
+
+template <typename Ty> std::string ParseAndEmit(llvm::StringRef YAML) {
+  Ty seq3;
+  Input yin(YAML);
+  yin >> seq3;
+  std::string out;
+  llvm::raw_string_ostream ostr(out);
+  Output yout(ostr);
+  yout << seq3;
+  return out;
+}
+
+TEST(YAMLIO, TestNestedSequence) {
+  {
+    llvm::StringRef Seq3YAML(R"YAML(---
+- - [ 1000, 1001 ]
+  - [ 1010, 1011 ]
+- - [ 1100, 1101 ]
+  - [ 1110, 1111 ]
+...
+)YAML");
+
+    std::string out = ParseAndEmit<NestedIntSeq3>(Seq3YAML);
+    EXPECT_EQ(out, Seq3YAML);
+  }
+
+  {
+    llvm::StringRef Seq3YAML(R"YAML(---
+- - - '000'
+    - '001'
+  - - '010'
+    - '011'
+- - - '100'
+    - '101'
+  - - '110'
+    - '111'
+...
+)YAML");
+
+    std::string out = ParseAndEmit<NestedStringSeq3>(Seq3YAML);
+    EXPECT_EQ(out, Seq3YAML);
+  }
+
+  {
+    llvm::StringRef Seq3YAML(R"YAML(---
+Seq3:
+  - - - '000'
+      - '001'
+    - - '010'
+      - '011'
+  - - - '100'
+      - '101'
+    - - '110'
+      - '111'
+...
+)YAML");
+
+    std::string out = ParseAndEmit<MappedStringSeq3>(Seq3YAML);
+    EXPECT_EQ(out, Seq3YAML);
+  }
+}
 
 //===----------------------------------------------------------------------===//
 //  Test dynamic typing