[PATCH] D92478: [yaml2obj/obj2yaml] - Make fields of the `Symbol` to be optional.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In general terms, I think we should have the following test cases for each field:

1. That a non-default value can be specified explicitly. If the value is an enum (like binding), each possible value should be specified.
2. That `=<none>` can be used where relevant, and that if it is, the value is whatever the default is.
3. That the field can be omitted entirely and the value is whatever the default is.

I don't think you need to test that `=<none>` values can be overridden. However, if you do override them, I wouldn't use the default value as the overriding value, personally. So, for example, with symbol binding, I'd override with STB_WEAK or STB_GLOBAL, rather than STB_LOCAL, to show that the default is not used still.



================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:668
                                   StringRef StrTable, ELFYAML::Symbol &S) {
-  S.Type = Sym->getType();
-  S.Value = Sym->st_value;
-  S.Size = Sym->st_size;
+  if (Sym->getType() != ELF::STT_NOTYPE)
+    S.Type = (ELFYAML::ELF_STT)Sym->getType();
----------------
Don't we need obj2yaml testing for these changes?


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

https://reviews.llvm.org/D92478