[PATCH] D66886: [yaml2obj][obj2yaml] - Use a single "Other" field instead of "Other", "Visibility" and "StOther".

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lib/ObjectYAML/ELFYAML.cpp:857
+    if (*Original != 0) {
+      UnknownFlagsHolder = std::to_string(*Original);
+      Ret.push_back(UnknownFlagsHolder);
----------------
jhenderson wrote:
> llvm::to_string I think is preferred.
`to_string` returns an allocated string => `Other`'s type can't be Optional<std::vector<StringRef>>

See below.


================
Comment at: test/tools/obj2yaml/elf-symbol-visibility.yaml:14
+# CHECK-NEXT:   - Name: internal
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:      - STV_INTERNAL
----------------
jhenderson wrote:
> Is there any way straightforward way of forcing obj2yaml to produce output in the form `Other: [ STV_HIDDEN ]` etc?
Not straightforward but I managed to find an approach..

```
 struct NormalizedOther {
+  struct Piece {
+    std::string Str;
+  };
 ...
-  Optional<std::vector<StringRef>> Other;
+  Optional<std::vector<Piece>> Other;
   std::string UnknownFlagsHolder;
 };
 } // namespace
 
+template<>
+struct ScalarTraits<NormalizedOther::Piece> {
+  static void output(const NormalizedOther::Piece &Val, void *, raw_ostream &Out) {
+    Out << Val.Str;
+  }
+  static StringRef input(StringRef Scalar, void *, NormalizedOther::Piece &Val) {
+    Val.Str = Scalar.str();
+    return {};
+  }
+  static QuotingType mustQuote(StringRef) { return QuotingType::None; }
+};
+template<> struct SequenceElementTraits<NormalizedOther::Piece> {
+  static const bool flow = true;
+};
```

This way obj2yaml will dump: `Other: [ STV_HIDDEN ]`


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

https://reviews.llvm.org/D66886