Crash when using natural order for subcommand options

[jtojnar]

The docs say:

Git style subcommands are created by either cli.command("cmd"), which changes the cli objects context to the command, or with opt.command("cmd"), which changes the command that option is for.

However when I try that:

static auto & command = Dim::Cli().command("apple").desc("Change color of the apple.");
static auto & color = command.opt<string>("color", "red");

I get a segfault:

#0  0x00007f2e553ac193 in std::__detail::_List_node_base::_M_hook(std::__detail::_List_node_base*) () from /lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x000055799bf8087d in std::__cxx11::list<std::unique_ptr<Dim::Cli::OptBase, std::default_delete<Dim::Cli::OptBase> >, std::allocator<std::unique_ptr<Dim::Cli::OptBase, std::default_delete<Dim::Cli::OptBase> > > >::_M_insert<std::unique_ptr<Dim::Cli::OptBase, std::default_delete<Dim::Cli::OptBase> > > (this=0x7ffcab754a50, __position=...) at /usr/include/c++/10/bits/stl_list.h:1912
#2  0x000055799bf7dcff in std::__cxx11::list<std::unique_ptr<Dim::Cli::OptBase, std::default_delete<Dim::Cli::OptBase> >, std::allocator<std::unique_ptr<Dim::Cli::OptBase, std::default_delete<Dim::Cli::OptBase> > > >::push_back (this=0x7ffcab754a50, __x=...) at /usr/include/c++/10/bits/stl_list.h:1217
#3  0x000055799bf562cf in Dim::Cli::addOpt (this=0x7ffcab754990, src=...) at _deps/dimcli-src/libs/dimcli/cli.cpp:1273
#4  0x000055799bf48caa in Dim::Cli::addOpt<Dim::Cli::Opt<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > (this=0x7ffcab754990, ptr=...)
    at _deps/dimcli-src/libs/dimcli/cli.h:885
#5  0x000055799bf47df3 in Dim::Cli::opt<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, void, void> (this=0x7ffcab754990, value=0x0, names=..., def=...) at _deps/dimcli-src/libs/dimcli/cli.h:811
#6  0x000055799bf47747 in Dim::Cli::opt<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7ffcab754990, names=..., def=...) at _deps/dimcli-src/libs/dimcli/cli.h:856
#7  0x000055799bf46b69 in __static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535) at ../main.cpp:9
#8  0x000055799bf46d0e in _GLOBAL__sub_I__Z5appleRN3Dim3CliE () at ../main.cpp:26
#9  0x000055799bf8e45d in __libc_csu_init ()
#10 0x00007f2e54fc04f1 in __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
#11 0x000055799bf462ee in _start ()

Reproducible example: https://github.com/jtojnar/repro/tree/f742c8ef00ae5f9677a65ebce29201cfa52918c7/dimcli-natural-static

[gknowles]

Thanks for the report!

Unlike Opt<>'s, capturing a Cli object by reference doesn't give you something you can use, instead you could do:

static auto command = Dim::Cli().command("apple").desc("Change color of the apple.");
static auto & color = command.opt<string>("color", "red");

or:

static Dim::Cli s_cli;
static auto & command = s_cli.command("apple").desc("Change color of the apple.");
static auto & color = command.opt<string>("color", "red");

or even:

static auto & color = Dim::Cli().command("apple")
    .desc("Change color of the apple.")
    .opt<string>("color", "red");

I agree that this is surprising and hard to diagnose. It should be improved so that capturing a Cli() by reference either:

1. works
2. asserts about what's wrong
3. or at least, has a hard to missing warning in the docs and code comments.

It currently behaves this way because when you construct a Cli() you are making a handle to the global configuration, and when it goes out of scope it no longer points at anything. Where as what is returned by cli.opt() is a reference to an object that continues to live in the global configuration.

I will investigate further.

[gknowles]

I pushed a fix, going with number 4, if you try you get a compile error.

auto & c1 = Dim::Cli(); // compile error, it's illegal to bind a temporary to a non-const reference

// BEFORE
// command(...) returned a self referencing Cli& hiding that it's a temporary, leading to a crash
auto & c2 = Dim::Cli().command("cmd"); 

// NOW
// command(...) and similar methods called on a temporary return Cli&& propagating it's status as a 
// temporary. And trying to bind it to a non-const reference gets you the same compile error as for 
// c1 above.
auto & c2 = Dim::Cli().command("cmd"); 

Will add coverage tests and figure out what to do with the docs in a couple of days.