[PATCH] D89463: [yaml2obj][ELF] - Simplify the code that performs sections validation.

[George Rimar via Phabricator via llvm-commits]

grimar marked 3 inline comments as done.
grimar added a comment.

Post commit comments are addressed in rG3be2b0d1a1e0b94e8586f695e8174f139f7a84d8 <https://reviews.llvm.org/rG3be2b0d1a1e0b94e8586f695e8174f139f7a84d8>



================
Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:198
+  // vector of <entry name, is used> pairs. It is used for sections validation.
+  virtual std::vector<std::pair<StringRef, bool>> getEntries() const {
+    return {};
----------------
jhenderson wrote:
> Maybe rather than a vector of pairs, a `StringMap<bool>` might be more straightforward. Not sure if order is an issue though.
I think the order might be an issue as we build and then check errors we report. I am not sure anout using map,
as this method is only useful for `validate()` where we want to iterate over all entries and never to lookup a particular name.
If you really want, I think I can replace it with `MapVector<StrinfRef, bool>` though.


================
Comment at: llvm/lib/ObjectYAML/DWARFYAML.cpp:311
     return "Entries and Content can't be used together";
-  return StringRef();
+  return "";
 }
----------------
jhenderson wrote:
> MaskRay wrote:
> > Seems inconsistent with `{}` you use below..
> FWIW, I'd prefer `""` throuhgout since it's more readable (you don't have to refer to the return type to figure out that it's a string-like thing), but don't feel strongly about it.
I've switched to using `""` in other places too for consistency. Perhaps, for string types it is a more natural empty value than `{}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89463