Conversation
The behavior changed around the variadic template constructor. It matches copy-constructors and so the latter are no longer auto-generated.
This was a false-positive from a temporary ElementArrayView
| // such as sc.empty are fast and do not call constructors and initialize | ||
| // memory --- in particular since this is also used internally for | ||
| // initialization the output in the implementation of many operations. | ||
| ASSERT_TRUE(std::is_pod_v<time_point>); |
There was a problem hiding this comment.
| vector(const vector &other) = default; | ||
| vector(vector &&other) noexcept = default; | ||
| vector &operator=(const vector &other) = default; | ||
| vector &operator=(vector &&other) noexcept = default; |
There was a problem hiding this comment.
These are needed because the variadic constructor below matches copy-constructors but its implementation does not work in that case. It seems like the compiler no longer generates a copy-constructor in this case.
The const / non-const overloads for the copy-consdtructor are needed because given only
vector(const vector &other) = default;
template <class... Args>
vector(Args &&...args) : data(std::forward<Args>(args)...) {}and calling it with a non-const lvalue, the variadic constructor provides a bettar match than the const copy-constructor.
| for (scipp::index i = 0; i < indices.dims().volume(); ++i) { | ||
| const auto &[begin, end] = indices.values<index_pair>()[i]; | ||
| const auto values = indices.values<index_pair>(); | ||
| const auto &[begin, end] = values[i]; |
There was a problem hiding this comment.
This and other similar cases are about avoiding a false-positive lifetime warning from returning references from a temporary ElementArrayView (return value of .values()).
|
Could you also test wheel builds as a sanity check? Something like a14b8ec should trigger the builds. |
| if(NOT DEFINED CMAKE_CXX_STANDARD) | ||
| set(CMAKE_CXX_STANDARD 17) | ||
| set(CMAKE_CXX_STANDARD 23) | ||
| endif(NOT DEFINED CMAKE_CXX_STANDARD) |
There was a problem hiding this comment.
Is it (currently at least) still possible to compile with 20 by settings this by hand? Or are we using any 23 features?
There was a problem hiding this comment.
It is possible by switching it in lib/CMakeLists.txt. Not sure if you can mix standards by changing it only in the package test. But nobody uses that anyway.
3 participants
See #3725