[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:877-880
+    llvm::WithColor::error()
+        << "an unknown value is used for symbol's 'Other' field: " << Name
+        << ".\n";
+    exit(1);
----------------
jhenderson wrote:
> Do other yaml2obj/obj2yaml error messages end with a full stop?
> 
> Also, this error reporting method here makes me uncomfortable, since we're in the middle of a library. Is there any sensible way to propagate the error back up the stack?
In ELFYAML.h

```
  template <>
  struct MappingTraits<ELFYAML::Symbol> {
  ...
+   std::string Err;
```

In ELFYAML.cpp,

```
  StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
                                                   ELFYAML::Symbol &Symbol) {
+  if (!Err.empty())
+    return Err;
...
  }

-   NormalizedOther(IO &IO, Optional<uint8_t> Original) : YamlIO(IO) {
+  NormalizedOther(IO &IO, ELFYAML::Symbol &Sym) : YamlIO(IO) {

-  MappingNormalization<NormalizedOther, Optional<uint8_t>> Keys(IO, Symbol.Other);
+  MappingNormalization<NormalizedOther, ELFYAML::Symbol> Keys(IO, Symbol);
```

I haven't verified if this approach works, if it does, the error can be propagated back via `validate()`.


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

https://reviews.llvm.org/D66886