From 9a149beb76ceca164204ef8653be47e404d19936 Mon Sep 17 00:00:00 2001 From: Vladimir Prus Date: Fri, 6 May 2005 06:40:39 +0000 Subject: [PATCH] Fix 'unknown option' error for two successive calls to 'store'. The bug triggered if - we store two parsed_option object into variables_map - the options descriptions associated with those parsed_option objects are different - an option present in first parser_option object is not declared in second options_description. The problem was that on the second 'store' call we went over all stored options, trying to get their description from the *second* options description object. Thanks to Hartmut Kaiser for the bug report. [SVN r28685] --- .../boost/program_options/variables_map.hpp | 9 ++++ src/variables_map.cpp | 35 ++++++++-------- test/variable_map_test.cpp | 41 +++++++++++++++++++ 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/include/boost/program_options/variables_map.hpp b/include/boost/program_options/variables_map.hpp index 2865e5c..ee31ecf 100644 --- a/include/boost/program_options/variables_map.hpp +++ b/include/boost/program_options/variables_map.hpp @@ -14,6 +14,7 @@ #include #include +#include namespace boost { namespace program_options { @@ -119,6 +120,14 @@ namespace boost { namespace program_options { /** Implementation of abstract_variables_map::get which does 'find' in *this. */ const variable_value& get(const std::string& name) const; + + /** Names of option with 'final' values -- which should not + be changed by subsequence assignments. */ + std::set m_final; + + friend void store(const basic_parsed_options& options, + variables_map& xm, + bool utf8); }; /** Stores in 'm' all options that are defined in 'options'. diff --git a/src/variables_map.cpp b/src/variables_map.cpp index 719a605..761b5db 100644 --- a/src/variables_map.cpp +++ b/src/variables_map.cpp @@ -23,6 +23,9 @@ namespace boost { namespace program_options { void store(const parsed_options& options, variables_map& xm, bool utf8) { + // TODO: what if we have different definition + // for the same option name during different calls + // 'store'. assert(options.description); const options_description& desc = *options.description; @@ -30,23 +33,7 @@ namespace boost { namespace program_options { // variables_map. Ehmm.. messy. std::map& m = xm; - // The set of existing values that should not be changed. - std::set final; - for (map::iterator k = m.begin(); - k != m.end(); - ++k) - { - if (!k->second.defaulted()) { - // TODO: what if we have different definition - // for the same option name during different calls - // 'store'. - // FIXME: consider the 'false'. - bool composing = desc.find(k->first, false).semantic()->is_composing(); - - if (!composing) - final.insert(k->first); - } - } + std::set new_final; // First, convert/store all given options for (size_t i = 0; i < options.options.size(); ++i) { @@ -57,7 +44,7 @@ namespace boost { namespace program_options { continue; // If option has final value, skip this assignment - if (final.count(name)) + if (xm.m_final.count(name)) continue; // Ignore options which are not described @@ -72,6 +59,7 @@ namespace boost { namespace program_options { // Explicit assignment here erases defaulted value v = variable_value(); } + try { d.semantic()->parse(v.value(), options.options[i].value, utf8); } @@ -81,7 +69,18 @@ namespace boost { namespace program_options { throw; } v.m_value_semantic = d.semantic(); + + // The option is not composing, and the value is explicitly + // provided. Ignore values of this option for subsequent + // calls to 'store'. We store this to a temporary set, + // so that several assignment inside *this* 'store' call + // are allowed. + if (!d.semantic()->is_composing()) + new_final.insert(name); } + xm.m_final.insert(new_final.begin(), new_final.end()); + + // Second, apply default values. const vector >& all = desc.options(); diff --git a/test/variable_map_test.cpp b/test/variable_map_test.cpp index ffa0494..4b71ab9 100644 --- a/test/variable_map_test.cpp +++ b/test/variable_map_test.cpp @@ -227,13 +227,54 @@ void test_priority() BOOST_CHECK_EQUAL(vm["include"].as< vector >()[1], 7); } +void test_multiple_assignments_with_different_option_description() +{ + // Test that if we store option twice into the same variable_map, + // and some of the options stored the first time are not present + // in the options descrription provided the second time, we don't crash. + options_description desc1(""); + desc1.add_options() + ("help,h", "") + ("includes", value< vector >()->composing(), ""); + ; + + options_description desc2(""); + desc2.add_options() + ("output,o", ""); + + vector input1; + input1.push_back("--help"); + input1.push_back("--includes=a"); + parsed_options p1 = command_line_parser(input1).options(desc1).run(); + + vector input2; + input1.push_back("--output"); + parsed_options p2 = command_line_parser(input2).options(desc2).run(); + + vector input3; + input3.push_back("--includes=b"); + parsed_options p3 = command_line_parser(input3).options(desc1).run(); + + + variables_map vm; + store(p1, vm); + store(p2, vm); + store(p3, vm); + + BOOST_REQUIRE(vm.count("help") == 1); + BOOST_REQUIRE(vm.count("includes") == 1); + BOOST_CHECK_EQUAL(vm["includes"].as< vector >()[0], "a"); + BOOST_CHECK_EQUAL(vm["includes"].as< vector >()[1], "b"); + +} int test_main(int, char* []) { test_variable_map(); test_semantic_values(); test_priority(); + test_multiple_assignments_with_different_option_description(); return 0; }