[PATCH] D18499: [ELF] - Implemented prototype of location counter support.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:98-99
@@ +97,4 @@
+void LinkerScript::fixUpLocations(std::vector<OutputSectionBase<ELFT> *> &S) {
+  // FIXME: for now this code just puts all orphan sections to the end of
+  // Locations list. That is not correct, linker can place such sections in
+  // according to predefined rules. It is very well described at
----------------
ruiu wrote:
> Although it is technically correct that what we are doing here is different from the doc, I don't think it necessarily mean it needs to be marked as "FIXME". My impression is that if you control section layout using linker scripts, you need to give complete layout. So please remove "FIXME" label from this comment and just state the fact:
> 
> Orphan sections are sections present in the input files which are not explicitly placed into the output file by the linker script. We place orphan sections at end of file. Other linkers places them using some heuristics as described in https://sourceware.org/binutils/docs/ld/Orphan-Sections.html#Orphan-Sections.
>> My impression is that if you control section layout using linker scripts, you need to give complete layout. 
Perfect, done ! I was thinking about the same often. For me it was always a bit wierd that linker do something on his own when script is given. That complicates the whole things at the same time privides no strict rules, but only heuristics, what can be confusing.
I was not sure that is acceptable to change that behavior though.

================
Comment at: ELF/LinkerScript.h:69
@@ +68,3 @@
+  template <class ELFT>
+  void fixUpLocations(std::vector<OutputSectionBase<ELFT> *> &);
+  template <class ELFT>
----------------
ruiu wrote:
> fixUp -> fixup
Done.


http://reviews.llvm.org/D18499