native/cpp/CommonCpp/DatabaseManagers/entities/Nullable.h
15 ↗	(On Diff #33480)	We should use `&&` here to enforce that `unique_ptr` is moved and value kept by this `unique_ptr` can't be used by other code after `Nullable` is constructed.
24 ↗	(On Diff #33480)	Am I correct that this line forces `T` to have default constructor that takes zero arguments? If so, then can we guarantee that whatever we have to put in `Nullable` conforms to this condition?

This revision now requires changes to proceed.Nov 22 2023, 5:25 AM

In D9902#290231, @ashoat wrote:

I'm guessing you've already considered std::optional?

Yes, I did.

There is no support for std::optional yet, but I found an issue on Emsctipen link and PR link. I tried solutions suggested there but two things:

It works fine when returning std::optional from C++ to JS, in the opposite direction it doesn't work that well - we need both.
This struct in JS we can use directly cons x = { isNull: false, value: 'asdf' }, while std::optional needs to be constructed (assuming it works) using constructor, the deleted, etc, which make JS side code more complicated.

I need to check whether we should use && or & in the constructor

kamil requested review of this revision.Nov 27 2023, 3:28 AM

kamil added inline comments.

native/cpp/CommonCpp/DatabaseManagers/entities/Nullable.h
15 ↗	(On Diff #33480)	I don't think so, here is why: The `std::unique_ptr<T>& ptr` means that `ptr` is a reference to a `std::unique_ptr`. When `ptr` is used, it dereferences the `unique_ptr`, and it give us the actual object that the `unique_ptr` is managing (an instance of type T), but the `std::unique_ptr` continues to own it. In this case, in the constructor, we use the copy constructor of T to initialize the value (not copy of pointer, copy of instance pointer points to). Even with something like `value = ptr;` we should be safe because the copy assignment operator of T should be called, and `value` gets copied and a distinct instance of T. The issue could be when `T value;` in line 9 will be defined as `T &value;` but this I believe should be caught by the compiler.
24 ↗	(On Diff #33480)	Am I correct that this line forces `T` to have default constructor that takes zero arguments? Yes, that was my intention If so, then can we guarantee that whatever we have to put in `Nullable` conforms to this condition? Not sure if I understand the question, but I believe the answer is: yes, it's done by C++ itself at compile time, there is no additional work we need to do