Skip to content
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.

feature request: support move only types #7

Open
Dushistov opened this issue Mar 1, 2018 · 4 comments
Open

feature request: support move only types #7

Dushistov opened this issue Mar 1, 2018 · 4 comments

Comments

@Dushistov
Copy link
Contributor

It would be nice to support types with deleted copy constructor/assign operator,
for AsyncFuture::Observable and AsyncFuture::Deferred instantation, for example:

struct Foo {
	explicit Foo(std::string s): s_{s} {}
	Foo(const Foo &) = delete;
	Foo &operator=(const Foo &) = delete;
	Foo(Foo &&o): s_{std::move(o.s_)} { }
	Foo &operator=(Foo &&o) {
		s_ = std::move(o.s_);
		return *this;
	}
...
};
@benlau
Copy link
Owner

benlau commented Mar 2, 2018

Hello @Dushistov,

Do you have any reference material about the usage of this kind of data structure?

@Dushistov
Copy link
Contributor Author

Hello @benlau,

Do you have any reference material about the usage of this kind of data structure?

The obvious examples are from std c++ library: std::unique_ptr and std::fstream.

I suppose generally these are the types that can not have copy/assigment with good and obvious semantic.

Personaly I wrap C API with C++ classes. And C APIs do not have copy functions, only allocation and deallocation. So my C++ classes all have disabled assigment/copy and only have move constructor and move assigment operators.

Currently my workaround would be creation of std::shared_ptr and moving my objects to/from it while I am using asyncfuture. Since I have many such "move only" classes, it is kind of boring and troublesome, and when I realized that I started to create one more library above asyncfuture, I decided to create an issue.

If it is troublesome to implement real support of move only, then may be usage of QSharedPointer / std::shared_ptr with overload via std::enable_if + !std::is_copy_constructible && std::is_move_constructible would be realy nice.

@Dushistov
Copy link
Contributor Author

And may be as good start will be:

    void complete(T value)
    {
        deferredFuture->complete(value);
    }

    void complete(QList<T> value) {
        deferredFuture->complete(value);
    }

implementation of perfect forwarding or may be just deferredFuture->complete(std::move(value)), to prevent extra copy.

This should give benefits not only for "move only" classes, but also for common classes like std::vector/std::string and so on.

@benlau
Copy link
Owner

benlau commented Apr 12, 2018

hi @Dushistov,

Sorry for late reply. I did a test case to validate the behaviour of move only types support by QFuture. Seem that it is not supported by QFuture yet. That is the code:

void Spec::test_move_only_type()
{
    struct MoveOnly {
        explicit MoveOnly(std::string s): s_{s} {}

        MoveOnly(const MoveOnly &) = delete;

        MoveOnly &operator=(const MoveOnly &) = delete;

        MoveOnly(MoveOnly &&o): s_{std::move(o.s_)} { }

        MoveOnly &operator=(MoveOnly &&o) {
            s_ = std::move(o.s_);
            return *this;
        }
        std::string s_;
    };

    QFuture<MoveOnly> future;
    auto t = future.result();
}

The compilation error in the line of future.result()

/Users/benlau/Development/Qt-Full/5.10.0/clang_64/lib/QtCore.framework/Headers/qfuture.h:157: error: call to deleted constructor of 'MoveOnly'
    return d.resultReference(0);
           ^~~~~~~~~~~~~~~~~~~~

If move only type is not supported by QFuture natively , then I have no method to make it works with AsyncFuture.

However, the suggestion of using std::move in complete should be a good idea. I will handle it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants