[PATCH] D26283: [ELF] - Partial support of --gdb-index command line option (Part 3).

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1441
+      Builder.readPubNamesAndTypes();
+  for (std::pair<StringRef, uint8_t> &Pair : NamesAndTypes) {
+    uint32_t Hash = hash(Pair.first.data());
----------------
clayborg wrote:
> use std::tie here with appropriate var names since you did it below?
I am not sure what you mean, sorry. Do you meant next form ?

  for (auto& p : someInitializingFunction()) {
    std::tie(a, b) = p;
    // do stuff;
  }

If so I am probably missing for what.


================
Comment at: ELF/SyntheticSections.cpp:1442
+  for (std::pair<StringRef, uint8_t> &Pair : NamesAndTypes) {
+    uint32_t Hash = hash(Pair.first.data());
+    size_t Offset = StringPool.add(Pair.first);
----------------
clayborg wrote:
> I am guessing we can guarantee that the StringRef is NULL terminated here since the string probably originated from a string table.
Yes, and hash() relies on that. Do you want to change hash() signature ?


https://reviews.llvm.org/D26283