Avoid using Template argument deduction for Vector

[umangyadav]

CTAD may not be supported by older C++ runtimes e.g. (gcc-7).

Seeing this error with -Werror on SLES builds :

../../../src/onnx/parse_resize.cpp:323:47: error: 'vector' may not intend to support class template argument deduction [-Werror,-Wctad-maybe-unsupported]
            auto vvv_ind = std::vector(n_dim, std::vector(2, std::vector<size_t>(out_elements)));
                                              ^
/usr/lib64/gcc/x86_64-suse-linux/7/../../../../include/c++/7/bits/stl_vector.h:216:11: note: add a deduction guide to suppress this warning
    class vector : protected _Vector_base<_Tp, _Alloc>
          ^
../../../src/onnx/parse_resize.cpp:323:28: error: 'vector' may not intend to support class template argument deduction [-Werror,-Wctad-maybe-unsupported]
            auto vvv_ind = std::vector(n_dim, std::vector(2, std::vector<size_t>(out_elements)));
                           ^
/usr/lib64/gcc/x86_64-suse-linux/7/../../../../include/c++/7/bits/stl_vector.h:216:11: note: add a deduction guide to suppress this warning
    class vector : protected _Vector_base<_Tp, _Alloc>
          ^

[lakhinderwalia]

(Approved, since you are explicitly stating the template type, but there may be more than one such instance of code.. I wonder, and this isn't upto the expected c++ 17, so definitely not optimal.)

However, please check of that version of compiler (gcc 7.x) has an upgrade for SLES.
And also if some specific compiler option could be turned on..

[umangyadav]

but there may be more than one such instance of code

I've enabled -Werror to catch those in CI.

compiler (gcc 7.x) has an upgrade for SLES

SLES docker had gcc 7.5. I am not sure when it is due for an upgrade in QA or DevOps' docker. But I agree that it should be upgraded, it is old.

[lakhinderwalia]

SLES docker had gcc 7.5. I am not sure when it is due for an upgrade in QA or DevOps' docker. But I agree that it should be upgraded, it is old.

I think gcc 7.x supports CTAD. This warning might be worth ignoring, as an alternative.

[codecov]

Codecov Report

Merging #2276 (030ed1f) into develop (bd425d0) will increase coverage by 0.00%.
Report is 1 commits behind head on develop.
The diff coverage is 100.00%.

❗ Current head 030ed1f differs from pull request most recent head e3324b3. Consider uploading reports for the commit e3324b3 to get more accurate results

@@           Coverage Diff            @@
##           develop    #2276   +/-   ##
========================================
  Coverage    91.50%   91.50%           
========================================
  Files          430      430           
  Lines        16133    16134    +1     
========================================
+ Hits         14762    14763    +1     
  Misses        1371     1371           

Files	Coverage Δ
src/onnx/parse_resize.cpp	95.33% <100.00%> (+0.03%)	⬆️

[pfultz2]

I am not sure when it is due for an upgrade in QA or DevOps' docker. But I agree that it should be upgraded, it is old.

We need to support the C++ runtime that applications are using which will be the C++ runtime used by the distro, in the case for sles its gcc 7. If we do upgrade the version it needs to use the one provided by the distro that is binary compatible with gcc 7 runtime. I do know rhel provides newer gcc versions, but I dont know about sles.

[pfultz2]

I think gcc 7.x supports CTAD.

It does, but the C++ runtime doesn't provide the deduction guidelines.

This warning might be worth ignoring, as an alternative.

We shouldn't ignore this warning, This is useful in diagnosing hard to figure out bugs due to a simple typo:

template<class It>
bool is_palindrome(It first, It last) {
    return std::equal(
        first, last,
        // Typo, should be std::make_reverse_iterator
        std::reverse_iterator(last),
        std::reverse_iterator(first)
    );
}

In general for container and container-like types(like std::tuple) it best to avoid ctad as well since it breaks generic code.