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

[Nick Lewycky via llvm-commits]

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