[PATCH] D69558: [ObjectYAML] - Redefine LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::Hex*) as LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added a comment.

In D69558#1725533 <https://reviews.llvm.org/D69558#1725533>, @MaskRay wrote:

> BTW, have you tested lldb? It recently gets some YAML tests.


Hah, my configuration doesn't build it. I fixed a few tests there before by doing a text search and replace,
this scheme works well at least for now. I checked that it doesn't use Hex8/Hex32/Hex64 types though, so it shouldn't fail.



================
Comment at: llvm/test/ObjectYAML/MachO/DWARF2-AddrSize8-FormValues.yaml:441
 #CHECK:             - Value:           0x000000000000000A
-#CHECK:               BlockData:       
-#CHECK:                 - 0x01
-#CHECK:                 - 0x02
-#CHECK:                 - 0x03
-#CHECK:                 - 0x04
-#CHECK:                 - 0x05
-#CHECK:                 - 0x06
-#CHECK:                 - 0x07
-#CHECK:                 - 0x08
-#CHECK:                 - 0x09
-#CHECK:                 - 0x00
+#CHECK:               BlockData:       [ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+#CHECK:                                  0x08, 0x09, 0x00 ]
----------------
MaskRay wrote:
> Just a thought: there are 7 elements on one line. 8 may look slightly better if we can make the column limit softer..
> 
> The default WrapColumn used in llvm::yaml::Output::Output is 70. YAMLTraits tries to wrap the line when the column number exceeds 70.
> 
>   lang=cpp
>   bool Output::preflightFlowElement(unsigned, void *&) {
>     if (NeedFlowSequenceComma)
>       output(", ");
>     if (WrapColumn && Column > WrapColumn) {
>       output("\n");
>       for (int i = 0; i < ColumnAtFlowStart; ++i)
>         output(" ");
>       Column = ColumnAtFlowStart;
>       output("  ");
>     }
>     return true;
>   }
> 
Yeah. And we probably might remove the excessive identation too. I.e. make obj2yaml produce the output which is often requested during reviews anyways. This would save some space for softening of this limit here and in other possible places.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69558/new/

https://reviews.llvm.org/D69558