[PATCH] D19977: [ELF] - Move section factory out from writer to make it reusable.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Fri May 6 02:50:57 PDT 2016


grimar added a comment.

In http://reviews.llvm.org/D19977#423160, @ruiu wrote:

> This is probably towards the right direction, but let's hold on until http://reviews.llvm.org/D19976 is settled.


Just in case:
If we land this one and http://reviews.llvm.org/D19981, then http://reviews.llvm.org/D19976 patch after rebase will be significantly reduced in size,
making it easier to review (and support).


================
Comment at: ELF/OutputSections.h:617-621
@@ -615,1 +616,7 @@
 
+// This class knows how to create an output section for a given
+// input section. Output section type is determined by various
+// factors, including input section's sh_flags, sh_type and
+// linker scripts.
+template <bool Is64Bits> struct SectionKey {
+  typedef typename std::conditional<Is64Bits, uint64_t, uint32_t>::type uintX_t;
----------------
ruiu wrote:
> This class (actually a struct) doesn't know how to create output sections. You have moved the comment to a wrong place.
Fixed.

================
Comment at: ELF/OutputSections.h:622
@@ +621,3 @@
+template <bool Is64Bits> struct SectionKey {
+  typedef typename std::conditional<Is64Bits, uint64_t, uint32_t>::type uintX_t;
+  StringRef Name;
----------------
ruiu wrote:
> I'd type it to ELFT rather than Is64Bits.
This patch just moves existent code without changes (at least that is what I expect from it). Improving factory relative code is another story and should be done separately I believe.


http://reviews.llvm.org/D19977





More information about the llvm-commits mailing list