Add a move constructor (may not work)

[ghost]

So I'm trying to return my app from a function instead of doing everything in main for testing purposes. But when I return the CLI::App instance from the function, I get an error.

Steps to reproduce:

#include <CLI/CLI.hpp>

CLI::App get_app() {
    CLI::App app { "CLI App"};
    return app;
}

int main(int argc, char **argv) {
    auto app = get_app();

    CLI11_PARSE(app, argc, argv);
}

I couldn't find the copy constructor defined anywhere, is that the problem? I know when a destructor is custom-defined, the default copy constructor is not included. Please help!

[phlptp]

I believe that the internal use of unique_ptr for several of the objects prohibits copy construction, and the virtual destructor is needed for the polymorphism in place on App objects. When I have constructed the app in a helper function the most straightforward and cleanest way is to return by shared ptr.

#include <CLI/CLI.hpp>

std::shared_ptr<CLI::App> get_app() {
   auto app=std::make_shared<CLI::App>("CLI App");
    return app;
}

int main(int argc, char **argv) {
    auto app = get_app();

    CLI11_PARSE(*app, argc, argv);
}

[henryiii]

You should also be able to use std::move or guaranteed copy elision (C++17). But yes, copy is not supported (and it would be wasteful/error prone). I don’t think std::move is tested currently.

[henryiii]

C++17 copy elision works, but it is probably too simple for real use: https://wandbox.org/permlink/QwadeQBBtebP1edT

Looks like App needs a move constructor for moves.

[henryiii]

Yes, adding App(App&&) = default; allows this to be moved. TODO.

[phlptp]

move constructors can be problematic with the virtual calls. I am guessing most of the use cases for polymorphism it wouldn't be an issue since there is no additional data added. but you could easily imagine problematic moves from a base class pointer that does something unexpected.

[henryiii]

Do you have an little example or a link to something about the problem? In a little test I couldn't get an issue. Assuming this is a form of slicing, which is a problem with copy constructors, but you can't keep a reference to the original object after a move. And you can't move across types.

Is this the problem: https://wandbox.org/permlink/k33iMM4jItFkmemf ?

I think we are safe as long as CLI11 doesn't internally use moves on pointers to App (which it doesn't currently)?

[phlptp]

The biggest issue I have experienced is usually returning a polymorphic object by value from a function then using it to construct a base class object with a move constructor or move assignment which typically causes the extra data and class information to get discarded. Usually doesn't cause any segmentation fault or any other errors since everything is still valid, just a sometimes unintentional loss of information, which can lead to some tough to track down errors. So as a general rule if the class is used polymorphically on a typical basis I generally don't allow move constructors and if I do they need to be explicit. In the case of CLI it would probably be ok since I don't imagine too many use cases where it would matter much since there are very few virtual functions. but you should make sure the move constructor is explicit at least.

// This file is a "Hello, world!" CLI11 program

#include <iostream>
#include <utility>

struct A
{
    A(int a) : a_var(a) {}
    int a_var;
    virtual ~A() = default;
    A(A&&) = default;
    virtual void print() const
    {
    std::cout<<"A="<<a_var<<'\n';
    }
};

struct B : public A
{
    using A::A;
    B(B&&) = default;
    B(int a, int b) : A(a), b_var(b) {}
    int b_var;
    
    virtual void print() const override
    {
    std::cout<<"B="<<b_var;
    A::print();
    }
};

B generate()
{
    return B(10,11);
}


int main()
{
    auto g1=generate();
    A g2(generate());
    g1.print();
    g2.print();

    return 0;
}

[phlptp]

actually not sure explicit works with default move constructors. didn't seem to do anything in wandbox

[ghost]

I got undefined behavior when accessing variables inside callbacks when I added the move constructor. :'(

[henryiii]

I assume the variables are somewhere that doesn't get destroyed? If they are part of a class or global, how are you keeping them alive? Could you add a small example of what fails?