[PATCH] D22915: [ELF] - Linkerscript: implemented SIZEOF(section)

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 28 14:08:50 PDT 2016


grimar added inline comments.

================
Comment at: ELF/LinkerScript.h:154
@@ -152,2 +153,3 @@
 
+  OutputSectionFactory<ELFT> *Factory = nullptr;
   uintX_t Dot;
----------------
ruiu wrote:
> I don't think it's beautiful as you are holding a pointer to a factory just to get sizes of sections. Factory is too powerful to be here.
> 
> LinkerScript creates output sections. Why don't you save it instead?
> 
> Also, as I described in https://reviews.llvm.org/D22740, this is yet another example that you are accessing something other than Dot to evaluate expressions. I do not see a reason to handle Dot as a special case. If you like to pass all variables as parameters, why don't you pass it as a Expr's parameter?
I thought that holding factory can help to simplify. We can get rid of arguments in methods in some pending patches and use this factory member instead.
LinkerScript creates some output sections, but that is the same what factory knows already, so I supposed it it better to reuse it than save what we created.
Anyways I just realized that factory is not enough either. I need to get somehow all sections including those which are added in addPredefinedSections(). So I guess I need to store Writer::OutputSections pointer instead. I`ll think what can be done here tomorrow.



https://reviews.llvm.org/D22915





More information about the llvm-commits mailing list