<div dir="ltr">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.<div><br></div><div>Cheers,</div><div>Lang.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 23, 2019 at 2:04 PM Nick Lewycky via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On 2019-04-23 1:52 p.m., Bjorn Pettersson via Phabricator via <br>
llvm-commits wrote:<br>
> bjope added inline comments.<br>
><br>
><br>
> ================<br>
> Comment at: llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp:161<br>
> +      auto &SL = KV.second;<br>
> +      std::sort(SL.ContentSections.begin(), SL.ContentSections.end(),<br>
> +                CompareByOrdinal);<br>
> ----------------<br>
> 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.<br>
<br>
It might help to prevent this pattern being reintroduced if it had a <br>
check in clang-tidy. There's already checks for other LLVM codebase <br>
specifics such as llvm-twine-local. Maybe someday we could have <br>
Phabricator + clang-tidy integration, or at perhaps something like a <br>
clang-tidy checking buildbot or the scan-build report but for clang-tidy.<br>
<br>
[1] - <a href="https://clang.llvm.org/extra/clang-tidy/checks/llvm-twine-local.html" rel="noreferrer" target="_blank">https://clang.llvm.org/extra/clang-tidy/checks/llvm-twine-local.html</a><br>
[2] - <a href="https://llvm.org/reports/scan-build/" rel="noreferrer" target="_blank">https://llvm.org/reports/scan-build/</a><br>
<br>
Nick<br>
<br>
> (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)<br>
><br>
><br>
> Repository:<br>
>    rL LLVM<br>
><br>
> CHANGES SINCE LAST ACTION<br>
>    <a href="https://reviews.llvm.org/D58704/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D58704/new/</a><br>
><br>
> <a href="https://reviews.llvm.org/D58704" rel="noreferrer" target="_blank">https://reviews.llvm.org/D58704</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>