[PATCH] D17120: ELF: Implement the correct semantics of .[cd]tors.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:802
@@ +801,3 @@
+    return false;
+  return S.drop_back(3).endswith("crtbegin");
+}
----------------
davide wrote:
> Can you name '3' instead of using the magic number?
We can name it SuffixLen or something like that, but it is probably too narrow. Added a comment instead.

================
Comment at: ELF/OutputSections.cpp:835
@@ +834,3 @@
+    return Y.empty();
+  return X < Y;
+}
----------------
davide wrote:
> Drive by comment: not a huge fan of X and Y as variable names. Sure, their scope is narrow but if you can come up with something more self-explicative, it would be better.
I rather prefer this way. I actually tried to name NameA and NameB, but X and Y grows on me as I modified this code many times before sending for the code review.

================
Comment at: ELF/OutputSections.cpp:842
@@ +841,3 @@
+template <class ELFT> void OutputSection<ELFT>::sortCtorsDtors() {
+  std::stable_sort(Sections.begin(), Sections.end(), compCtors<ELFT>);
+  reassignOffsets();
----------------
davide wrote:
> It is not obvious to me why you need stable sorting. Do you mind adding a comment?
Added a comment to the function for init/fini.


http://reviews.llvm.org/D17120