[PATCH] D24604: [ELF] - Implemented --sort-section cmd line option and SORT_NONE script command.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:970-972
@@ -969,3 +969,5 @@
     return SortByAlignment;
+  if (skip("SORT_NONE"))
+    return SortDisabled;
   return SortNone;
 }
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > ruiu wrote:
> > > > This naming scheme is confusing. SORT_NONE is mapped to SortDisabled, and SortNone doesn't mean SORT_NONE?
> > > Yes, a bit confusing, but according to documentation SORT_NONE is used to _disable_ sorting. A agree that probably better to map it to SortNone, but what is better name for absence of sorting (which is none) ? SortEmpty probably ?
> > I don't know. Null, Empty, Void, Dont, etc... Nothing feels right. I thought about it for a few minutes but didn't come up with a good name. Maybe just return 0?
> It is probably a bit strange to return 0 when we have enum. Also problem is that we use the code like that:
> 
> ```
>     if (Cmd->SortInner)
>       std::stable_sort(V.begin(), V.end(), getComparator(Cmd->SortInner));
>     if (Cmd->SortOuter)
>       std::stable_sort(V.begin(), V.end(), getComparator(Cmd->SortOuter));
> ```
> 
> So in that case for this to work we will need to modify start value of enum:
> 
> ```
> enum SortKind { SortDisabled = 1, SortByName, SortByAlignment };
> ```
> 
> What is not very beautiful IMO. I`ll think more about that.
If you don't come up with a good name, I don't think `= 1` is ugly.


https://reviews.llvm.org/D24604