Repository navigation
rsx: fix some warnings - #17687
rsx: fix some warnings#17687
Conversation
| std::memcpy(that._data, _data, size_bytes()); | ||
| std::memcpy(_data, _stack_alloc, that.size_bytes()); | ||
| } | ||
| else |
There was a problem hiding this comment.
i think it makes more sense to add trivially_copyable as a Ty constraint on the entire class. None of the other methods cater to the edge case of non-copyables and the user should probably use std::vector for those.
is there a piece of code actually using this path at the moment?
There was a problem hiding this comment.
My reasoning is that, the container was already written to hyper-optimize common operations without typical C++ restrictions such as this one and the destructible one to speed up push/pop/swap.
There was a problem hiding this comment.
I added the constraint.
Maybe I still have to add a static cast to fix the warning, we'll see
There was a problem hiding this comment.
I guess it only works for windows xD
/rpcs3/rpcs3/Emu/RSX/Common/simple_array.hpp:19:52: note: 'std::pair<unsigned int, bool>' is not trivially copyable
/rpcs3/rpcs3/Emu/RSX/Common/simple_array.hpp:19:52: note: because it has a user provided copy assignment operator
/rpcs3/rpcs3/Emu/RSX/Common/simple_array.hpp:19:52: note: because it has a user provided move assignment operator
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_pair.h:187:12: note: 'pair<unsigned int, bool>' defined here
187 | struct pair
There was a problem hiding this comment.
Add an exception for std::pair and std::tuple. This is a known silly thing with std::pair and std::tuple.
Now that I think about it, this was the reason I didn't add the constraint in the first place.
There was a problem hiding this comment.
But then again that won't help with the "warning" despite it being harmless. The whole class has a lot of 'trust me I know what I'm doing' scenarios, especially before we had proper concepts for defining constraints. All I know is that component-wise transfer does not belong here.
Option2 - we roll a custom pair<T, U> only possible when T and U are builtin types. That should cover 99% of valid pair usages in the code which is to just avoid creating small structs to hold result values.
There was a problem hiding this comment.
At this point we might reimplement std::pair and not use std::tuple with simple_array.
There was a problem hiding this comment.
std::pair also has savestate serialization issues for the same reason so maybe we should implement a replacement class.
There was a problem hiding this comment.
Huh, I wasn't aware it was such a widespread pain-point but it makes sense to roll a custom one in that case.
Let's leave the constraint in place and roll a utils::pair and utils::tuple at least for use with basic types and go from there.
b7d2bf8 to
c48640c
Compare
ce17007 to
8c76980
Compare
No description provided.