This is an archive of the discontinued LLVM Phabricator instance.

[ADT] drop_begin: use adl_begin/adl_end. NFC.
ClosedPublic

Authored by Meinersbur on Jun 26 2018, 10:19 AM.

Details

Summary

The instantiation of the drop_begin function template usually fails because the functions begin() and end() do not exist. Only when using on a container from the std namespace (or llvm::iterator_ranges of something derived from std::iterator), they are matched to std::begin() and std::end() due to Koenig-lookup.

Explicitly use llvm::adl_begin and llvm::adl_end to make drop_begin applicable to anything iterable (including C-style arrays).

A solution for general llvm::iterator_ranges was already tried in r244620, but got reverted in r244621 due to MSVC not liking it.

Diff Detail

Event Timeline

Meinersbur created this revision.Jun 26 2018, 10:19 AM
Meinersbur added inline comments.Jun 26 2018, 10:21 AM
include/llvm/ADT/iterator_range.h
62–65

Space was added by clang-format.

I think we now have adl_begin/adl_end helpers in the llvm namespace that
should do the right thigh (which is that the call should be made like
"using std::begin; ...begin(X)..." The same way swap is meant to be called
(this allows a default implementation in the std namespace and allows for
types to implement their own custom implementation in their own namespace
to be found via ADL))

Interesting, I didn't know about adl_begin/adl_end/adl_swap. Thanks. Will update the diff soon.

  • Use adl_begin/adl_end
Meinersbur retitled this revision from [ADT] drop_begin: use std namespace. NFC. to [ADT] drop_begin: use adl_begin/adl_end. NFC..Jun 26 2018, 10:41 AM
Meinersbur edited the summary of this revision. (Show Details)
dblaikie accepted this revision.Jun 26 2018, 1:52 PM

Looks good - thanks!

This revision is now accepted and ready to land.Jun 26 2018, 1:52 PM
This revision was automatically updated to reflect the committed changes.