[PATCH] D19171: [ELF] - Get rid of SectionOrder array.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:172
@@ -163,5 +171,3 @@
 int LinkerScript::compareSections(StringRef A, StringRef B) {
-  auto E = SectionOrder.end();
-  auto I = std::find(SectionOrder.begin(), E, A);
-  auto J = std::find(SectionOrder.begin(), E, B);
-  if (I == E || J == E)
+  uint32_t I = getSectionOrder(A);
+  uint32_t J = getSectionOrder(B);
----------------
grimar wrote:
> So compareSections() is used to sort OutputSections.
> Later linkerscript uses this order in fixupLocations() to add orphans.
> 
> Therefore Locations vector is updated at the very end, 
> before actual address assignment.
> 
> Another variant would be to call fixupLocations() earlier, before compareSections().
> Then orpan order would be script driven, but will add some code complexity as will need export of
> fixupLocations() method.
> 
> I like both approach. First (this patch) for simple code, second for giving 
> all cards to script to control orphans order.
Makes sense, but in that case (uint32_t)-1 is not an error code, but it actually mean the sorting order of the given section name (e.g. the lowest priority). In that case (uint32_t)-1 looks odd. Please use UINT32_MAX instead because it's what you really mean. Also it needs a comment about when UINT32_MAX returns.

Also, there's no reason to use uint32_t. I'd use int and INT_MAX.


Repository:
  rL LLVM

http://reviews.llvm.org/D19171