This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][NFC] Use range-based STL wrappers
ClosedPublic

Authored by Amir on Jun 19 2022, 10:26 PM.

Details

Summary

Replace std:: algorithms taking begin/end iterators with llvm:: counterparts
accepting ranges.

Diff Detail

Event Timeline

Amir created this revision.Jun 19 2022, 10:26 PM
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Amir requested review of this revision.Jun 19 2022, 10:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2022, 10:26 PM
Amir retitled this revision from [BOLT][NFC] Use range-based STL wrappers to [BOLT][NFC][WIP] Use range-based STL wrappers.Jun 19 2022, 10:35 PM
Amir updated this revision to Diff 438532.Jun 20 2022, 7:55 PM

Keep std::reverse

Amir retitled this revision from [BOLT][NFC][WIP] Use range-based STL wrappers to [BOLT][NFC] Use range-based STL wrappers.Jun 21 2022, 8:43 AM

It's a bit confusing to call a function named "stable_sort", which is a well-known C++ stdlib function, but we're actually calling stable_sort from LLVM and not from stdlib. I've tried looking up in LLVM repo and there are all sorts of usages of is_contained, for example: some of them use llvm:is_contained, others use is_contained directly. What is your take on this? Should we call llvm::stable_sort, or just stable_sort?

Amir added a comment.Jun 21 2022, 6:08 PM

It's a bit confusing to call a function named "stable_sort", which is a well-known C++ stdlib function, but we're actually calling stable_sort from LLVM and not from stdlib. I've tried looking up in LLVM repo and there are all sorts of usages of is_contained, for example: some of them use llvm:is_contained, others use is_contained directly. What is your take on this? Should we call llvm::stable_sort, or just stable_sort?

I’ll make it fully qualified to avoid ambiguity.

Amir updated this revision to Diff 439183.Jun 22 2022, 3:29 PM

Remaining llvm::

This revision is now accepted and ready to land.Jun 23 2022, 2:52 PM
This revision was automatically updated to reflect the committed changes.