[PATCH] D24604: [ELF] - Implemented --sort-section cmd line option and SORT_NONE script command.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 15 11:27:21 PDT 2016
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
More information about the llvm-commits
mailing list