Hello MXNet devs, I'd like to discuss uniformly adopting C++11 range loops in the MXNet project. The benefits I see are:
* Improved C++ readability (examples below). * Consistency with other languages. The range-loops are quite similar to loops almost all other programming languages. Given we're a project that supports many languages this language consistency could be positive for our community. * Consistency within the same project. Currently different authors have different loops styles which hurts codebase readability. * Best available performance. There are often multiple ways to write loops in C++ with subtle differences in performance and memory usage between loop methods. Using range-loops ensures we get the best possible perf using an intuitive loop pattern. * Slightly lower chance for bugs / OOB accesses when dealing with indexing in an array for example. If we decide to enable this uniformly throughout the project we can enable this policy with a simple clang-tidy configuration change. There would be no need for reviewers to have to manually provide feedback when someone uses an older C++ loops style. -Kellen Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/ Previous clang-tidy discussion on the list: https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E ------------------------- Examples: for (auto axis_iter = param.axis.begin() ; axis_iter!= param.axis.end(); ++axis_iter) { CHECK_LT(*axis_iter, static_cast<int>(ishape.ndim())); stride_[reverse_index] = ishape[*axis_iter]; ... --> for (int axis : param.axis) { CHECK_LT(axis, static_cast<int>(ishape.ndim())); stride_[reverse_index] = ishape[axis]; ... -------------------------- for (size_t i = 0; i < in_array.size(); i++) { auto &nd = in_array[i]; pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype()); } --> for (auto & nd : in_array) { pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype()); }
