jarro2783 / cxxopts

Lightweight C++ command line option parser
MIT License
4.25k stars 590 forks source link

Why parsing empty text in parse_value ? #366

Closed mwestphal closed 2 years ago

mwestphal commented 2 years ago

in template <typename T> void parse_value(const std::string& text, std::vector<T>& value) there is a branch for empty text like this:

      if (text.empty()) {
        T v;
        parse_value(text, v);
        value.emplace_back(std::move(v));
        return;
      }    

Why would we ever want to parse empty text ? Why even emplacing back a value in the vector with an empty text ?

This seems to fails and throw in certain cases.

jarro2783 commented 2 years ago

This allows an empty string when an option can have multiple values. Otherwise the code after it tries to split text by some delimiter which fails when it is empty.

mwestphal commented 2 years ago

But shouldn't an empty string results in an empty vector? Also with T=double, parse_value throw an exception.

jarro2783 commented 2 years ago

No, values are added into a vector. So --value "" adds an empty string to the values for --value.

mwestphal commented 2 years ago

but with T=double, how do you convert the empty string ? to a zero ?

jarro2783 commented 2 years ago

well then that's probably invalid input. Do you have an example that shows this doing the wrong thing?

mwestphal commented 2 years ago

It is a bit involved inside a bigger application, I need to take it out in a simple example.

jarro2783 commented 2 years ago

If you can just give the expected options and some input that fails that should be enough.

mwestphal commented 2 years ago

Are the callstack of the sigabrt:

Thread 1 "f3d" received signal SIGABRT, Aborted.
0x00007ffff78a149c in ?? () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff78a149c in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff7851958 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff783b53d in abort () from /usr/lib/libc.so.6
#3  0x00007ffff7a99833 in __gnu_cxx::__verbose_terminate_handler () at /usr/src/debug/gcc/libstdc++-v3/libsupc++/vterminate.cc:95
#4  0x00007ffff7aa5cfc in __cxxabiv1::__terminate (handler=<optimized out>) at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:48
#5  0x00007ffff7aa5d69 in std::terminate () at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:58
#6  0x00007ffff7aa5fcd in __cxxabiv1::__cxa_throw (obj=<optimized out>, tinfo=0x555555685680 <typeinfo for cxxopts::argument_incorrect_type>, 
    dest=0x55555559d6dc <cxxopts::argument_incorrect_type::~argument_incorrect_type()>) at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_throw.cc:98
#7  0x000055555559d778 in cxxopts::throw_or_mimic<cxxopts::argument_incorrect_type> (text="") at /home/glow/dev/f3d/src/application/cxxopts.hpp:540
#8  0x000055555560d400 in cxxopts::values::stringstream_parser<double> (text="", value=@0x7fffffffda38: 4.6355705639563209e-310) at /home/glow/dev/f3d/src/application/cxxopts.hpp:929
#9  0x000055555560cbbb in cxxopts::values::parse_value<double, (void*)0> (text="", value=@0x7fffffffda38: 4.6355705639563209e-310) at /home/glow/dev/f3d/src/application/cxxopts.hpp:975
#10 0x000055555560c9ce in cxxopts::values::parse_value<double> (text="", value=std::vector of length 0, capacity 0) at /home/glow/dev/f3d/src/application/cxxopts.hpp:988

So the call is a simple as cxxopts::values::parse_value<double> (text="", value=std::vector of length 0, capacity 0)

Value is an empty vector of double, and cxxopts gets an empty string to parse into the vector. I expect the vector to stay empty and especially to not throw.

Hth

jarro2783 commented 2 years ago

Can you give me a bit further up the stack so that I can see what is calling it? It would help if I knew the command line that it was trying to parse.

mwestphal commented 2 years ago

Here is the full cxxopts stack:

#0  0x00007ffff78a149c in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff7851958 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff783b53d in abort () from /usr/lib/libc.so.6
#3  0x00007ffff7a99833 in __gnu_cxx::__verbose_terminate_handler () at /usr/src/debug/gcc/libstdc++-v3/libsupc++/vterminate.cc:95
#4  0x00007ffff7aa5cfc in __cxxabiv1::__terminate (handler=<optimized out>) at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:48
#5  0x00007ffff7aa5d69 in std::terminate () at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:58
#6  0x00007ffff7aa5fcd in __cxxabiv1::__cxa_throw (obj=<optimized out>, tinfo=0x555555686660 <typeinfo for cxxopts::argument_incorrect_type>, 
    dest=0x55555559e99c <cxxopts::argument_incorrect_type::~argument_incorrect_type()>) at /usr/src/debug/gcc/libstdc++-v3/libsupc++/eh_throw.cc:98
#7  0x000055555559ea38 in cxxopts::throw_or_mimic<cxxopts::argument_incorrect_type> (text="") at /home/glow/dev/f3d/src/application/cxxopts.hpp:540
#8  0x000055555560e6c0 in cxxopts::values::stringstream_parser<double> (text="", value=@0x7fffffffcd28: 4.6355705641934724e-310) at /home/glow/dev/f3d/src/application/cxxopts.hpp:929
#9  0x000055555560de7b in cxxopts::values::parse_value<double, (void*)0> (text="", value=@0x7fffffffcd28: 4.6355705641934724e-310) at /home/glow/dev/f3d/src/application/cxxopts.hpp:975
#10 0x000055555560dc8e in cxxopts::values::parse_value<double> (text="", value=std::vector of length 0, capacity 0) at /home/glow/dev/f3d/src/application/cxxopts.hpp:988
#11 0x000055555560c845 in cxxopts::values::abstract_value<std::vector<double, std::allocator<double> > >::parse (this=0x555556240700) at /home/glow/dev/f3d/src/application/cxxopts.hpp:1089
#12 0x000055555559767f in cxxopts::OptionValue::parse_default (this=0x5555562406c0, details=std::shared_ptr<const cxxopts::OptionDetails> (use count 2, weak count 0) = {...})
    at /home/glow/dev/f3d/src/application/cxxopts.hpp:1353
#13 0x0000555555598a17 in cxxopts::OptionParser::parse_default (this=0x7fffffffd150, details=std::shared_ptr<cxxopts::OptionDetails> (use count 2, weak count 0) = {...})
    at /home/glow/dev/f3d/src/application/cxxopts.hpp:2059
#14 0x0000555555599d08 in cxxopts::OptionParser::parse (this=0x7fffffffd150, argc=1, argv=0x7fffffffd268) at /home/glow/dev/f3d/src/application/cxxopts.hpp:2330
#15 0x00005555555992d8 in cxxopts::Options::parse (this=0x7fffffffd530, argc=1, argv=0x7fffffffd268) at /home/glow/dev/f3d/src/application/cxxopts.hpp:2188
#16 0x000055555558dc31 in ConfigurationOptions::GetOptions (this=0x5555556f57a0, appOptions=..., options=..., inputs=std::vector of length 0, capacity 0, 
mwestphal commented 2 years ago

the command line is basically empty (only the program name), and the option has a default, which is an empty vector.

jarro2783 commented 2 years ago

is the option set as vector<double> with a default value of an empty string?

mwestphal commented 2 years ago

Yes, exactly.

jarro2783 commented 2 years ago

How is your option set? I tried this, but I couldn't reproduce the problem.

("vector", "A list of doubles", cxxopts::value<std::vector<double>>()->default_value(""))
mwestphal commented 2 years ago

Here is a minimal example, keep in mind I'm not really experienced with cxxopts, just a standard user.

int main(int argc, char** argv)
{ 
  cxxopts::Options cxxOptions("name", "title");

  std::vector<double> testOption;
  auto grp0 = cxxOptions.add_options("group");
  auto val = cxxopts::value<std::vector<double>>(testOption);
  // val = val->default_value("0"); // This line behave as expected, outputs "0.0000"
  val = val->default_value(""); // This line throw an error, expected output "empty"
  grp0("test", "doc", val, "help");
  cxxOptions.parse(argc, argv);

  std::string out = testOption.empty() ? "empty" : std::to_string(testOption[0]);
  std::cout<<out<<std::endl;

  exit(EXIT_SUCCESS);
  }
jarro2783 commented 2 years ago

That's the problem, you can't set a default like that. You're effectively telling it to have a default value of a single double that is described by an empty string, which doesn't parse as a double.

mwestphal commented 2 years ago

Indeed, this is not about default value specifically but about the parsing of vector option. Reading the doc it seems to be covered by this point here:

Parsing of list of values in form of an std::vector is also supported, as long as T can be parsed

And since "" cant be parsed for double or int, it cant be parsed for std::vector double or std::vector int Of course, std::vector std::string can be parsed but it results not in an empty vector but in a vector of size 1 containing an empty string.

I understand the approach, but I also think that it would be nice to have a way to generate an empty vector using cxxopts.