This is an archive of the discontinued LLVM Phabricator instance.

ADT/STLExtras: Introduce llvm::empty(); NFC
ClosedPublic

Authored by MatzeB on Oct 30 2018, 4:09 PM.

Details

Summary

This is modeled after C++17 std::empty().

(I got talked into shaving this yak in D53885)

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB created this revision.Oct 30 2018, 4:09 PM

(maybe some unit tests?)

Also, what about having only a single implementation:

template <typename Range>
constexpr bool empty(const Range &r) {
  return adl_begin(r) == adl_end(r);
}

Not sure why the standard library doesn't do it this way, but there are probably good reasons... though maybe they don't apply to us/llvm?

(maybe some unit tests?)

Also, what about having only a single implementation:

template <typename Range>
constexpr bool empty(const Range &r) {
  return adl_begin(r) == adl_end(r);
}

Not sure why the standard library doesn't do it this way, but there are probably good reasons... though maybe they don't apply to us/llvm?

What if the container has an optimized empty() method that works faster than actually producing begin and end iterators?

MatzeB updated this revision to Diff 171837.Oct 30 2018, 4:48 PM
  • Add a simple unittest
  • Go with the adl_begin() == adl_end() implementation for now. We can always switch to more fine grained overloading when we actually deem it necessary for performance. And after all we will probably just replace it with std::empty() anyway once we use C++17 in llvm.
dblaikie accepted this revision.Oct 30 2018, 4:56 PM

Sure - let's go with your original solution (plus the test case) for forwards-compatibility with the standard.

This revision is now accepted and ready to land.Oct 30 2018, 4:56 PM

(oh, if you are going to switch back to the original implementation - probably put "empty" for iterator_range in iterator_range, rather than as a non-member)

This revision was automatically updated to reflect the committed changes.