[PATCH] D13958: Mere SHF_STRING

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 23 09:35:49 PDT 2015


ruiu added inline comments.

================
Comment at: ELF/InputSection.cpp:134
@@ +133,3 @@
+template <class ELFT>
+typename MergeInputSectionBase<ELFT>::uintX_t
+MergeInputSectionBase<ELFT>::getOffset(uintX_t Offset) {
----------------
Please add a comment saying that this function translates an offset in this input merged section to an offset in an output section.

================
Comment at: ELF/InputSection.cpp:142
@@ +141,3 @@
+
+  auto I = std::upper_bound(
+      this->Offsets.begin(), this->Offsets.end(), Offset,
----------------
It's nice to write a comment here to describe what this code is doing. As I understand, `Offsets` is a sorted list of starting offsets of all entries, so we first do binary search on that list to find the start and end positions of an entry that contains a given `Offset`, and then find that entry from the output section. Result is cached.

================
Comment at: ELF/InputSection.cpp:158
@@ +157,3 @@
+  OutputSectionBase<ELFT> *OutSec = this->OutSec;
+  if (isa<MergeStringInputSection<ELFT>>(this))
+    Base =
----------------
Why don't you use dyn_cast here?

================
Comment at: ELF/InputSection.cpp:162
@@ +161,3 @@
+  else
+    Base = static_cast<MergeOutputSection<ELFT> *>(OutSec)->getOffset(Entry);
+
----------------
and cast<>?


http://reviews.llvm.org/D13958





More information about the llvm-commits mailing list