[PATCH] D39094: [ELF] - Cleanup of processSectionCommands().

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/OutputSections.cpp:119-120
   }
+  if (Noload)
+    Type = SHT_NOBITS;
 
----------------
ruiu wrote:
> This function is `addSection`, which is supposed to add a section to an output section, right? Adding this piece of code which is not related to adding sections to this function is not a good idea. This is how this function organically grown (and I took quite a long time to clean it up), so when you add code to some function, please stop and think if what you are about to do is semantically correct.
This function not only adds section, but also initializes and changes the `Type`.
And that happened before my change already, so I assume it is proper place to set `Type` because
it probably desirable to change it in a single place of code, no ?

We can split this code (lines 97-120) to something like `setSectionType`, but that would be separate refactoring change.
Will such change be OK for you ?



https://reviews.llvm.org/D39094