[PATCH] D61190: [yaml2obj] - Make interface of `NameToIdxMap` class be human friendly and fix users.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 08:45:47 PDT 2019


jhenderson added a comment.

I'm happy with this in principle, but I do wonder how you made sure you caught all instances!



================
Comment at: tools/yaml2obj/yaml2elf.cpp:78
   }
-  /// asserts if name is not present in the map
+  /// asserts if name is not present in the map.
   unsigned get(StringRef Name) const {
----------------
As you're tidying this comment up, please change "asserts" to "Asserts".


================
Comment at: tools/yaml2obj/yaml2elf.cpp:400
       for (auto SecName : YamlPhdr.Sections) {
-        uint32_t Index = 0;
-        SN2I.lookup(SecName.Section, Index);
-        const auto &SHeader = SHeaders[Index];
+        const auto &SHeader = SHeaders[SN2I.get(SecName.Section)];
         PHeader.p_offset = std::min(PHeader.p_offset, SHeader.sh_offset);
----------------
What happens if you specify a section name that doesn't exist? Does this result in an assertion? If so, we should produce an actual error instead in this case.

Similar point may apply below.


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

https://reviews.llvm.org/D61190





More information about the llvm-commits mailing list