[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