mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kellen sunderland <>
Subject [DISCUSS] Use modernized C++11 range loops uniformly throughout the project
Date Fri, 28 Sep 2018 09:12:18 GMT
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
* 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.


Reference PR:
Previous clang-tidy discussion on the list:

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());

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message