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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 30 09:47:54 PDT 2019


grimar marked an inline comment as done.
grimar added a comment.

In D61190#1482558 <https://reviews.llvm.org/D61190#1482558>, @jhenderson wrote:

> I do wonder how you made sure you caught all instances!


I am usually using renaming magic for such things. I.e. I renamed `addName` to `addName1`, `lookup` to `lookup1` and so on,
then tried to compile and fixed all the places. Then renamed back.



================
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);
----------------
jhenderson wrote:
> 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.
Oh, I did not notice we do not fail if list a section that do not exist. I think we should check it earlier than here then.
What about landing the following refactoring then? D61322


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

https://reviews.llvm.org/D61190





More information about the llvm-commits mailing list