This is an archive of the discontinued LLVM Phabricator instance.

Rename llvm::array_lengthof into llvm::size
AbandonedPublic

Authored by serge-sans-paille on Jan 26 2022, 1:02 PM.

Details

Summary

llvm::array_lengthof matches the behavior of std::size from C++17, so I renamed it
and moved it to llvm/ADT/STLForwardCompat.h

For consistency, I also added llvm::size over any container with a size method,
which is also available in C++17.

There is, however, already an llvm::size available in llvm/ADT/STLExtras.h,
that's an extension of std::size over ranges with O(1) iterator difference. I've
kept this implementation in STLExtras.h because it's... an extra extension to
the standard.

Diff Detail

Event Timeline

serge-sans-paille requested review of this revision.Jan 26 2022, 1:02 PM
serge-sans-paille edited reviewers, added: bkramer; removed: jcranmer.Jan 26 2022, 1:03 PM

Most of the diff is just about renaming, the interesting part is in https://reviews.llvm.org/D118281#change-w0K8NZ5ah3FS

Interplay between the three overloads seems to be ok: https://godbolt.org/z/89xe9abda

mehdi_amini added inline comments.Jan 26 2022, 1:19 PM
llvm/include/llvm/ADT/STLForwardCompat.h
79

Because we already have a llvm::size in STLExtras and it'll remain there after we adopt a new standard, I'm not sure about introducing these right now: we won't be able to remove then and mass-update the uses with a textual replacement (it'd match the users of the STLExtras variant).

Considering this, what is the motivation to rename array_lengthof right now instead of waiting for LLVM to move to C++17?

Same. Can we just wait for set(CMAKE_CXX_STANDARD 17 ... and change these to std::size in one step, instead of having the llvm::size intermediate step?

serge-sans-paille abandoned this revision.Jan 27 2022, 3:06 AM

Ok, your arguments make sense. Let's be patient and wait for C++17.
I've set a smaller goal in https://reviews.llvm.org/D118342 :-)