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

[Lang Hames via llvm-commits]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190424/95ef862b/attachment.html>