[PATCH] D35386: Speed up gdb index creation

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/GdbIndex.cpp:56
+      continue;
+    if (auto *M = StringSwitch<LLDDWARFSection *>(Sec->Name)
+                      .Case(".debug_info", &InfoSection)
----------------
auto -> LLDDWARFSection


================
Comment at: ELF/GdbIndex.cpp:60
+                      .Case(".debug_line", &LineSection)
+                      .Default(nullptr)) {
+      M->Data = toStringRef(Sec->Data);
----------------
Is this safe?


================
Comment at: ELF/GdbIndex.cpp:65
+    }
+    if (auto *M = StringSwitch<StringRef *>(Sec->Name)
+                      .Case(".debug_abbrev", &AbbrevSection)
----------------
auto -> StringRef


================
Comment at: ELF/GdbIndex.cpp:69
+                      .Case(".debug_gnu_pubtypes", &GnuPubTypesSection)
+                      .Default(nullptr))
+      *M = toStringRef(Sec->Data);
----------------
Is returning a nullptr safe?

I think writing without `StringSwitch` is more straightforward:

  if (Sec->Name == ".debug_abbrev")
    AbbrevSection = toStringRef...;
  else if (Sec->Name == ".debug_gnu_pubnames")
    GnuPubNameSection = toStringRef...;
  else if ...


================
Comment at: ELF/GdbIndex.cpp:74
+
+template <class ELFT>
+template <class RelTy>
----------------
I'd write a function comment.


================
Comment at: ELF/GdbIndex.h:22-26
+struct LLDDWARFSection final : public llvm::DWARFSection {
+  InputSectionBase *Sec = nullptr;
+};
+
+template <class ELFT> class LLDDwarfObj final : public llvm::DWARFObj {
----------------
Are these classes designed to be inherited? I prefer delegation over inheritance and am wondering if this is a good approach.


================
Comment at: ELF/GdbIndex.h:41
+public:
+  LLDDwarfObj(elf::ObjectFile<ELFT> *Obj);
+  const llvm::DWARFSection &getInfoSection() const override {
----------------
explicit?


https://reviews.llvm.org/D35386