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]

include/boost/program_options/variables_map.hpp

@@ -14,6 +14,7 @@
 
 #include <string>
 #include <map>
+#include <set>
 
 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<std::string> m_final;
+
+        friend void store(const basic_parsed_options<char>& options, 
+                          variables_map& xm,
+                          bool utf8);
     };
 
     /** Stores in 'm' all options that are defined in 'options'. 

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<std::string, variable_value>& m = xm;
 
-        // The set of existing values that should not be changed.
-        std::set<std::string> final;
-        for (map<string, variable_value>::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<std::string> 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<shared_ptr<option_description> >& all = desc.options();

test/variable_map_test.cpp

@@ -227,13 +227,54 @@ void test_priority()
     BOOST_CHECK_EQUAL(vm["include"].as< vector<int> >()[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<string> >()->composing(), "");
+        ;
+
+    options_description desc2("");
+    desc2.add_options()
+        ("output,o", "");
+
+    vector<string> input1;
+    input1.push_back("--help");
+    input1.push_back("--includes=a");
+    parsed_options p1 = command_line_parser(input1).options(desc1).run();
+
+    vector<string> input2;
+    input1.push_back("--output");
+    parsed_options p2 = command_line_parser(input2).options(desc2).run();
+
+    vector<string> 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<string> >()[0], "a");
+    BOOST_CHECK_EQUAL(vm["includes"].as< vector<string> >()[1], "b");
+
+} 
 
 int test_main(int, char* [])
 {
     test_variable_map();
     test_semantic_values();
     test_priority();
+    test_multiple_assignments_with_different_option_description();
     return 0;
 }