[PATCH] D58704: Initial (incomplete) implementation of JITLink - A replacement for RuntimeDyld.

[David Blaikie via llvm-commits]

I don't think there's any great/complex design decision here, or
performance implications - llvm::sort is just a wrapper that takes a
range instead of iterators so you don't have to call begin/end
manually (& risk passing mismatched iterators or anything else).

On Wed, Apr 24, 2019 at 3:16 PM Lang Hames via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> In this case the elements are guaranteed to have distinct keys. Do we offer any guidance in that case? If there's no overhead I am happy to use llvm::sort. Otherwise I can add a comment explaining why std::sort is legal here.
>
> Cheers,
> Lang.
>
> On Tue, Apr 23, 2019 at 2:04 PM Nick Lewycky via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> On 2019-04-23 1:52 p.m., Bjorn Pettersson via Phabricator via
>> llvm-commits wrote:
>> > bjope added inline comments.
>> >
>> >
>> > ================
>> > Comment at: llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp:161
>> > +      auto &SL = KV.second;
>> > +      std::sort(SL.ContentSections.begin(), SL.ContentSections.end(),
>> > +                CompareByOrdinal);
>> > ----------------
>> > This is a post commit comment, but I just wanted to inform that the CodingStandards.rst says "As a rule of thumb, always make sure to use llvm::sort instead of std::sort.". Afaik there has been some general cleanup to replace all std::sort uses by llvm::sort. It might still be ongoing for some projects but I think "llvm" was cleaned up some time ago.
>>
>> It might help to prevent this pattern being reintroduced if it had a
>> check in clang-tidy. There's already checks for other LLVM codebase
>> specifics such as llvm-twine-local. Maybe someday we could have
>> Phabricator + clang-tidy integration, or at perhaps something like a
>> clang-tidy checking buildbot or the scan-build report but for clang-tidy.
>>
>> [1] - https://clang.llvm.org/extra/clang-tidy/checks/llvm-twine-local.html
>> [2] - https://llvm.org/reports/scan-build/
>>
>> Nick
>>
>> > (I think main problem with std::sort is when having several equal keys, I haven't looked at the code here to see if it is a problem, I only noticed that some new uses of std::sort had popped up when browsing the code)
>> >
>> >
>> > Repository:
>> >    rL LLVM
>> >
>> > CHANGES SINCE LAST ACTION
>> >    https://reviews.llvm.org/D58704/new/
>> >
>> > https://reviews.llvm.org/D58704
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits