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

How to use JsonDocuments with std::unique_ptrs? #1678

Closed
matth-x opened this issue Nov 21, 2021 · 5 comments
Closed

How to use JsonDocuments with std::unique_ptrs? #1678

matth-x opened this issue Nov 21, 2021 · 5 comments
Labels
bug v6 ArduinoJson 6

Comments

@matth-x
Copy link

matth-x commented Nov 21, 2021

Hi Benoît,

Thank you for all the work on your library, it's really awesome and helps me a lot with my own library (if you're curious, you can have a look: https://github.com/matth-x/ArduinoOcpp).

As your design allows to build the JSON documents as a composite, I took full advantage of it. To make the memory management still straightforward despite multiple sub-documents per document, I would like to go on using unique_ptrs because of its familiar semantics.

But the following way of using them seems problematic:

std::unique_ptr<DynamicJsonDocument> doc = factory->makeDoc1_uniquePtr();

//...

if (specialCase) {
    doc = factory->makeDoc2_uniquePtr(); //compiler error
}

This gives me the following compiler output:


~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\unique_ptr.h: In instantiation of 'void std::unique_ptr<_Tp, _Dp>::reset(std::unique_ptr<_Tp, _Dp>::pointer) [with _Tp = ArduinoJson6185_91::BasicJsonDocument; _Dp = std::default_delete >; std::unique_ptr<_Tp, _Dp>::pointer = ArduinoJson6185_91::BasicJsonDocument*]':
~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\unique_ptr.h:251:7: 
  required from 'std::unique_ptr<_Tp, _Dp>& std::unique_ptr<_Tp, _Dp>::operator=(std::unique_ptr<_Tp, _Dp>&&) [with _Tp = ArduinoJson6185_91::BasicJsonDocument; _Dp = std::default_delete >]'
src/mytest.cpp:102:10:   required from here
~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\unique_ptr.h:342:6: 
error:** call of overloaded 'swap(ArduinoJson6185_91::BasicJsonDocument*&, ArduinoJson6185_91::BasicJsonDocument*&)' is ambiguous
  swap(std::get<0>(_M_t), __p);
      ^
In file included from ~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\stl_pair.h:59:0,
                 from ~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\stl_algobase.h:64,
                 from ~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\deque:60,
                 from src/mytest.h:8,
                 from src/mytest.cpp:5:
~\.platformio\packages\toolchain-xtensa32\xtensa-esp32-elf\include\c++\5.2.0\bits\move.h:176:5: note: 
candidate: void std::swap(_Tp&, _Tp&) [with _Tp = ArduinoJson6185_91::BasicJsonDocument*] 
     swap(_Tp& __a, _Tp& __b)
     ^
In file included from .pio/libdeps/esp32-development-board/ArduinoJson/src/ArduinoJson/MsgPack/endianess.hpp:8:0,
                 from .pio/libdeps/esp32-development-board/ArduinoJson/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp:9,       
                 from .pio/libdeps/esp32-development-board/ArduinoJson/src/ArduinoJson.hpp:37,
                 from .pio/libdeps/esp32-development-board/ArduinoJson/src/ArduinoJson.h:9,
                 from src/mytest.h:10,
                 from src/mytest.cpp:5:
.pio/libdeps/esp32-development-board/ArduinoJson/src/ArduinoJson/Polyfills/utility.hpp:11:13: note: candidate: void ArduinoJson6185_91::swap(T&, T&) [with T = ArduinoJson6185_91::BasicJsonDocument*]
 inline void swap(T& a, T& b) {
             ^
*** [.pio\build\esp32-development-board\src\mytest.cpp.o] Error 1

The problem seems to be due to the implementation of the operator= in the unique_ptr triggering a call to swap which could apply to both std::swap and ARDUINOJSON_NAMESPACE::swap as a consequence of ADL.

What is the right way of using JsonDocuments with unique_ptrs? Have I mistaken something? Thank you in advance for tips!

@bblanchon
Copy link
Owner

Hi @matth-x,

I just had a quick glance à ArduinoOcpp and this looks like a fantastic project; I'll add a link on arduinojson.org as soon as I can.

Honestly, I don't see the point of wrapping a DynamicJsonDocument in a std::unique_ptr, since this mechanics is already implemented in the class itself (it would make sense for a StaticJsonDocument, though).
However, this ambiguous call sounds like a bug, and I need to investigate further.

BTW, I think you might be interested in #1343 for creating composite documents.

Best regards,
Benoit

@matth-x
Copy link
Author

matth-x commented Nov 23, 2021

Hi Benoît,

Amazing, thank you for fixing it so quickly! And yes, I will follow #1343 and see when there will be updates.

If you like, I would also write a reference for your website. After having been working with your project for 2 years I have seen more than enough to recommend it.

@bblanchon
Copy link
Owner

By "reference", you mean "testimonial", right?
If so, you can write it here, or send me an email, as you prefer.

@matth-x
Copy link
Author

matth-x commented Dec 1, 2021

Here is what I would like to tell the community about ArduinoJson:

ArduinoJson is a good example for great engineering. If you rely heavily on JSON, then ArduinoJson will not only give you the tools to handle data processing efficiently, but it will also be a framework to build better products upon.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2022
@bblanchon
Copy link
Owner

The fix was release in ArduinoJson 6.19.0.

@bblanchon bblanchon added the v6 ArduinoJson 6 label Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug v6 ArduinoJson 6
Projects
None yet
Development

No branches or pull requests

2 participants