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

[Rafael Espíndola via llvm-commits]

How about NoSort? That way the enum represents what is written is the
script file.

Cheers,
Rafael


On 15 September 2016 at 14:27, George Rimar <grimar at accesssoftek.com> wrote:

> grimar added inline comments.
>
> ================
> Comment at: ELF/LinkerScript.cpp:970-972
> @@ -969,3 +969,5 @@
>      return SortByAlignment;
> +  if (skip("SORT_NONE"))
> +    return SortDisabled;
>    return SortNone;
>  }
> ----------------
> 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.
>
>
> https://reviews.llvm.org/D24604
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160916/3407e1a7/attachment.html>