-
Notifications
You must be signed in to change notification settings - Fork 77
Support move-semantics for UniValue #52
Comments
Note also that univalue seems to be not yet built with C++11 (at least it looked like that when I last played around). Is that by design (in which case this issue would be invalid) or just because it has not been changed yet? |
|
That makes sense. For move constructors and assignment, it would be easy to make them conditional on C++11 with some preprocessor macro. What do you think about that? If you think it makes sense, I can give it a try. |
@domob1812 Sounds good to me. The main difficulty I imagine will be crafting a test that proves the code compiles absent C++11 :) |
Ok, let me try. :) Yes, having an automated test for that is probably hard or impossible - but if all the new code is protected by preprocessor conditionals and we run a manual test or so, I hope it is still fine. (But let's just see what I can come up with.) |
My guess would be forcing the dialect via |
Any updates on that issue? Is c++98 still a requirement? My PR #79 introduces move semantics which can lead to a very significant speedup of about a factor of 2. |
I had a discussion with Jeff on this issue about 6 months ago. While some projects like Bitcoin Core and Bitcoin ABC support C++11 or later, there was a concern that Ubuntu 16 LTS and other distros have already shipped older versions of univalue and older compilers, making it difficult to assess impact on "in the field" users. However, Ubuntu 16 LTS is reaching EOL in a few months. This may be worth revisiting. |
UniValue
as a class could take advantage of C++11 move semantics to improve efficiency in some cases. (It is probably not very relevant in practice and for Bitcoin Core, but would still be nice to have.)What do you think, would that make sense to add? (If yes, then I'm happy to provide a PR for it.)
The text was updated successfully, but these errors were encountered: