[PATCH] D23213: Allow SmallVector to be used as a yaml sequence

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: include/llvm/Support/YAMLTraits.h:1362-1382
@@ -1361,1 +1361,23 @@
 
+template <typename T> struct SequenceTraitsImpl {
+  typedef typename T::value_type _type;
+  static size_t size(IO &io, T &seq) { return seq.size(); }
+  static _type &element(IO &io, T &seq, size_t index) {
+    if (index >= seq.size())
+      seq.resize(index + 1);
+    return seq[index];
+  }
+};
+
+template <typename T> struct FlowSequenceTraitsImpl {
+  typedef typename T::value_type _type;
+  static size_t size(IO &io, T &seq) { return seq.size(); }
+  static _type &element(IO &io, T &seq, size_t index) {
+    (void)flow; /* Remove this workaround after PR17897 is fixed */
+    if (index >= seq.size())
+      seq.resize(index + 1);
+    return seq[index];
+  }
+  static const bool flow = true;
+};
+
----------------
majnemer wrote:
> Is this formatted right? it looks funny to me for some reason.
It's clang-formatted, so I guess so.  Which party looks funny?

================
Comment at: include/llvm/Support/YAMLTraits.h:1381
@@ +1380,3 @@
+  }
+  static const bool flow = true;
+};
----------------
majnemer wrote:
> Where is flow used?
It seems to be used to distinguish a flow sequence from a regular sequence.  I don't know how it's used, other than to say that it's an optional member that if defined, causes the parser to treat it differently.  See line 47 of this file.

That said, `FlowSequenceTraitsImpl` and `SequenceTraitsImpl` are identical with the exception of the existence of the `flow` variable.  So I'm going to remove this class and consolidate.  New patch incoming shortly.

================
Comment at: include/llvm/Support/YAMLTraits.h:1406
@@ +1405,3 @@
+  namespace yaml {                                                             \
+  template <int N>                                                             \
+  struct SequenceTraits<SmallVector<_type, N>>                                 \
----------------
majnemer wrote:
> Shouldn't this be `unsigned` to match `SmallVector`'s template?
Yes, good catch.


https://reviews.llvm.org/D23213