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

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 19:07:39 PST 2016


davide added a subscriber: davide.
davide added a comment.

Some comments.


================
Comment at: ELF/OutputSections.cpp:802
@@ +801,3 @@
+    return false;
+  return S.drop_back(3).endswith("crtbegin");
+}
----------------
Can you name '3' instead of using the magic number?

================
Comment at: ELF/OutputSections.cpp:835
@@ +834,3 @@
+    return Y.empty();
+  return X < Y;
+}
----------------
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.

================
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();
----------------
It is not obvious to me why you need stable sorting. Do you mind adding a comment?


http://reviews.llvm.org/D17120





More information about the llvm-commits mailing list