Assorted fixes to for_each and fetch comments

Summary: [Folly] Assorted fixes to for_each and fetch comments

Reviewed By: aary

Differential Revision: D9632155

fbshipit-source-id: 64932770d3476cdd06c5fa9eb9bd845600f18fff

folly/container/Foreach-inl.h

@@ -41,9 +41,9 @@ using std::get;
 
 /**
  * The adl_ functions below lookup the function name in the namespace of the
- * type of the object being passed into the function.  If no function with
- * that name exists for the passed object then the default std:: versions are
- * going to be called
+ * type of the object being passed into the function. If no function with that
+ * name exists for the passed object then the default std:: versions are going
+ * to be called
  */
 template <std::size_t Index, typename Type>
 auto adl_get(Type&& instance) -> decltype(get<Index>(std::declval<Type>())) {
@@ -61,22 +61,21 @@ auto adl_end(Type&& instance) -> decltype(end(instance)) {
 } // namespace adl
 
 /**
- * Enable if the range supports fetching via non member get<>()
+ * Enable if the tuple supports fetching via non member get<>()
  */
 template <typename T>
 using EnableIfNonMemberGetFound =
     void_t<decltype(adl::adl_get<0>(std::declval<T>()))>;
 /**
- * Enable if the range supports fetching via a member get<>()
+ * Enable if the tuple supports fetching via a member get<>()
  */
 template <typename T>
 using EnableIfMemberGetFound =
     void_t<decltype(std::declval<T>().template get<0>())>;
 
 /**
- * A get that tries ADL get<> first and if that is not found tries to execute
- * a member function get<> on the instance, just as proposed by the structured
- * bindings proposal here 11.5.3
+ * A get that tries member get<> first and if that is not found tries ADL get<>.
+ * This mechanism is as found in the structured bindings proposal here 11.5.3.
  * http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf
  */
 template <std::size_t Index, typename Type, typename = void>
@@ -97,10 +96,7 @@ struct Get<Index, Type, EnableIfMemberGetFound<Type>> {
 };
 
 /**
- * Concepts-ish
- */
-/**
- * Check if the range is a tuple or a range
+ * Check if the sequence is a tuple
  */
 template <typename Type, typename T = typename std::decay<Type>::type>
 using EnableIfTuple = void_t<
@@ -108,7 +104,7 @@ using EnableIfTuple = void_t<
     decltype(std::tuple_size<T>::value)>;
 
 /**
- * Check if the range is a range
+ * Check if the sequence is a range
  */
 template <typename Type, typename T = typename std::decay<Type>::type>
 using EnableIfRange = void_t<
@@ -116,7 +112,7 @@ using EnableIfRange = void_t<
     decltype(adl::adl_end(std::declval<T>()))>;
 
 /**
- * Forwards the return value of the first element of the range, used to
+ * Forwards the return value of the first element of the sequence, used to
  * determine the type of the first element in the range in SFINAE use cases
  */
 template <typename Sequence, typename = void>
@@ -131,10 +127,10 @@ struct DeclvalSequence<Sequence, EnableIfTuple<Sequence>> {
 
 /**
  * Check if the functor accepts one or two arguments, one of the first element
- * in the range, assuming that all the other elements can also be passed to the
- * functor, and the second being an instantiation of std::integral_constant,
- * and the third being an instantiation of LoopControl, to provide
- * breakability to the loop
+ * in the sequence, assuming that all the other elements can also be passed to
+ * the functor, and the second being an instantiation of std::integral_constant,
+ * and the third being an instantiation of LoopControl, to provide breakability
+ * to the loop
  */
 template <typename Sequence, typename Func>
 using EnableIfAcceptsOneArgument = void_t<decltype(std::declval<Func>()(
@@ -175,7 +171,7 @@ using EnableIfHasIndexingOperator =
 
 /**
  * Implementation for the range iteration, this provides specializations in
- * the case where the function returns a break or continue.
+ * the case where the function returns a break or continue
  */
 template <typename Seq, typename F, typename = void>
 struct ForEachRange {
@@ -250,7 +246,7 @@ void for_each_range_impl(Sequence&& range, Func& func) {
 /**
  * The class provides a way to tell whether the function passed in to the
  * algorithm returns an instance of LoopControl, if it does then the break-able
- * implementation will be used.  If the function provided to the algorithm
+ * implementation will be used. If the function provided to the algorithm
  * does not use the break API, then the basic no break, 0 overhead
  * implementation will be used
  */
@@ -292,7 +288,7 @@ struct ForEachTupleImpl<Seq, F, EnableIfBreaksTuple<Seq, F>> {
  * The two top level compile time loop iteration functions handle the dispatch
  * based on the number of arguments the passed in function can be passed, if 2
  * arguments can be passed then the implementation dispatches work further to
- * the implementation classes above.  If not then an adaptor is constructed
+ * the implementation classes above. If not then an adaptor is constructed
  * which is passed on to the 2 argument specialization, which then in turn
  * forwards implementation to the implementation classes above
  */
@@ -323,11 +319,10 @@ void for_each_tuple_impl(Sequence&& seq, Func& func) {
 
 /**
  * Top level handlers for the for_each loop, the basic specialization handles
- * ranges and the specialized version handles compile time ranges (tuple like)
+ * tuples and the specialized version handles ranges
  *
- * This implies that if a range is a compile time range, its compile time
- * get<> API (whether through a member function or through a ADL looked up
- * method) will be used in preference over iterators
+ * This implies that if type is both a range and a tuple, it is treated as a
+ * range rather than as a tuple
  */
 template <typename R, typename = void>
 struct ForEachImpl {

folly/container/Foreach.h

@@ -26,7 +26,7 @@ namespace folly {
 /**
  * @function for_each
  *
- * folly::for_each is a generalized iteration algorithm.  Example:
+ * folly::for_each is a generalized iteration algorithm. Example:
  *
  *  auto one = std::make_tuple(1, 2, 3);
  *  auto two = std::vector<int>{1, 2, 3};
@@ -36,20 +36,24 @@ namespace folly {
  *  folly::for_each(one, func);
  *  folly::for_each(two, func);
  *
- * The for_each function allows iteration through sequences, these
- * can either be runtime sequences (i.e. entities for which std::begin and
- * std::end work) or compile time sequences (as deemed by the presence of
- * std::tuple_length<>, get<> (ADL resolved) functions)
+ * The for_each function allows iteration through sequences, these can either be
+ * runtime sequences (i.e. entities for which std::begin and std::end work) or
+ * compile time sequences (as deemed by the presence of std::tuple_length<> and
+ * member get<> or ADL get<> functions).
  *
- * The function is made to provide a convenient library based alternative to
- * the proposal p0589r0, which aims to generalize the range based for loop
- * even further to work with compile time sequences.
+ * If a sequence type is both a runtime sequence (aka range) and a compile-time
+ * sequence (aka tuple), then it is treated as a range in preference to a tuple.
+ * An example of such a type is std::array.
+ *
+ * The function is made to provide a convenient library based alternative to the
+ * proposal p0589r0, which aims to generalize the range based for loop even
+ * further to work with compile time sequences.
  *
  * A drawback of using range based for loops is that sometimes you do not have
- * access to the index within the range.  This provides easy access to that,
- * even with compile time sequences.
+ * access to the index within the range. This provides easy access to that, even
+ * with compile time sequences.
  *
- * And breaking out is easy
+ * And breaking out is easy:
  *
  *  auto range_one = std::vector<int>{1, 2, 3};
  *  auto range_two = std::make_tuple(1, 2, 3);
@@ -63,17 +67,16 @@ namespace folly {
  *  folly_for_each(range_one, func);
  *  folly_for_each(range_two, func);
  *
- * A simple use case would be when using futures, if the user was doing calls
- * to n servers then they would accept the callback with the futures like this
+ * A simple use case would be when using futures, if the user was doing calls to
+ * n servers then they would accept the callback with the futures like this:
  *
  *  auto vec = std::vector<std::future<int>>{request_one(), ...};
  *  when_all(vec.begin(), vec.end()).then([](auto futures) {
  *    folly::for_each(futures, [](auto& fut) { ... });
  *  });
  *
- * Now when this code switches to use tuples instead of the runtime
- * std::vector, then the loop does not need to change, the code will still
- * work just fine
+ * Now when this code switches to use tuples instead of the runtime std::vector,
+ * then the loop does not need to change, the code will still work just fine:
  *
  *  when_all(future_one, future_two, future_three).then([](auto futures) {
  *    folly::for_each(futures, [](auto& fut) { ... });
@@ -85,7 +88,7 @@ FOLLY_CPP14_CONSTEXPR Func for_each(Range&& range, Func func);
 /**
  * The user should return loop_break and loop_continue if they want to iterate
  * in such a way that they can preemptively stop the loop and break out when
- * certain conditions are met
+ * certain conditions are met.
  */
 namespace for_each_detail {
 enum class LoopControl : bool { BREAK, CONTINUE };
@@ -96,11 +99,11 @@ constexpr auto loop_continue = for_each_detail::LoopControl::CONTINUE;
 
 /**
  * Utility method to help access elements of a sequence with one uniform
- * interface
+ * interface.
  *
  * This can be useful for example when you are looping through a sequence and
  * want to modify another sequence based on the information in the current
- * sequence
+ * sequence:
  *
  *  auto range_one = std::make_tuple(1, 2, 3);
  *  auto range_two = std::make_tuple(4, 5, 6);
@@ -108,12 +111,12 @@ constexpr auto loop_continue = for_each_detail::LoopControl::CONTINUE;
  *    folly::fetch(range_two, index) = ele;
  *  });
  *
- * For non-tuple like ranges, this works by first trying to use the iterator
- * class if the iterator has been marked to be a random access iterator.  This
- * should be inspectable via the std::iterator_traits traits class.  If the
- * iterator class is not present or is not a random access iterator then the
- * implementation falls back to trying to use the indexing operator
- * (operator[]) to fetch the required element
+ * For ranges, this works by first trying to use the iterator class if the
+ * iterator has been marked to be a random access iterator. This should be
+ * inspectable via the std::iterator_traits traits class. If the iterator class
+ * is not present or is not a random access iterator then the implementation
+ * falls back to trying to use the indexing operator (operator[]) to fetch the
+ * required element.
  */
 template <typename Sequence, typename Index>
 FOLLY_CPP14_CONSTEXPR decltype(auto) fetch(Sequence&& sequence, Index&& index);