Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tmva] Fix warnings in TMVA #14265

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Conversation

guitargeek
Copy link
Contributor

More detail in the commit descriptions.

When compiling with gcc 13.2, there is a new warning in TMVA that is
fixed by this commit:

```c++
In static member function ‘static constexpr _Up* std::__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp = long unsigned int; _Up = long unsigned int; bool _IsMove = true]’,
    inlined from ‘constexpr _OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = true; _II = long unsigned int*; _OI = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_algobase.h:506:30,
    inlined from ‘constexpr _OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = true; _II = long unsigned int*; _OI = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_algobase.h:533:42,
    inlined from ‘constexpr _OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = true; _II = long unsigned int*; _OI = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_algobase.h:540:31,
    inlined from ‘constexpr _OI std::copy(_II, _II, _OI) [with _II = move_iterator<long unsigned int*>; _OI = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_algobase.h:633:7,
    inlined from ‘static _ForwardIterator std::__uninitialized_copy<true>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator<long unsigned int*>; _ForwardIterator = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_uninitialized.h:147:27,
    inlined from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = move_iterator<long unsigned int*>; _ForwardIterator = long unsigned int*]’ at /usr/include/c++/13.2.1/bits/stl_uninitialized.h:185:15,
    inlined from ‘constexpr _ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = move_iterator<long unsigned int*>; _ForwardIterator = long unsigned int*; _Tp = long unsigned int]’ at /usr/include/c++/13.2.1/bits/stl_uninitialized.h:373:37,
    inlined from ‘constexpr _ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = long unsigned int*; _ForwardIterator = long unsigned int*; _Allocator = allocator<long unsigned int>]’ at /usr/include/c++/13.2.1/bits/stl_uninitialized.h:399:2,
    inlined from ‘constexpr void std::vector<_Tp, _Alloc>::_M_range_insert(iterator, _ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = const long unsigned int*; _Tp = long unsigned int; _Alloc = std::allocator<long unsigned int>]’ at /usr/include/c++/13.2.1/bits/vector.tcc:819:9,
    inlined from ‘constexpr std::vector<_Tp, _Alloc>::iterator std::vector<_Tp, _Alloc>::insert(const_iterator, std::initializer_list<_Tp>) [with _Tp = long unsigned int; _Alloc = std::allocator<long unsigned int>]’ at /usr/include/c++/13.2.1/bits/stl_vector.h:1411:17,
    inlined from ‘void TMVA::MethodDL::ParseInputLayout()’ at /home/rembserj/spaces/master/root/src/root/tmva/tmva/src/MethodDL.cxx:470:24:
/usr/include/c++/13.2.1/bits/stl_algobase.h:437:30: error: ‘void* __builtin_memmove(void*, const void*, long unsigned int)’ forming offset 32 is out of the bounds [0, 32] [-Werror=array-bounds=]
  437 |             __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      |
```

The fix suggested in this commit is to directly assign from a full
initializer list, which is also more readable in the code.
@phsft-bot
Copy link

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@@ -467,11 +467,11 @@ void MethodDL::ParseInputLayout()
// when we will support 3D convolutions we would need to add extra 1's
if (inputShape.size() == 2) {
// case of dense layer where only width is specified
inputShape.insert(inputShape.begin() + 1, {1,1});
inputShape = {inputShape[0], 1, 1, inputShape[1]};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why this gives a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, sorry. It could even be a compiler bug, or maybe because the iterator is invalidated after the first insertion in case a re-allocation happens.

In any case, I think the new formulation of the logic is cleaner, so I wouldn't try harder to really understand this warning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, fine!

Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

Test Results

       10 files         10 suites   2d 0h 23m 13s ⏱️
  2 485 tests   2 485 ✔️ 0 💤 0
23 773 runs  23 773 ✔️ 0 💤 0

Results for commit ae3f0b4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants