roll back Group::Add when duplicate flag detection throws

args.hxx

@@ -1715,7 +1715,24 @@ namespace args
                 children.emplace_back(&child);
 
                 if(child.IsFlag()) {
+#ifndef ARGS_NOEXCEPT
+                    // Detection runs from the child's own constructor, so a
+                    // duplicate throws before that constructor completes and the
+                    // child's storage is released while the stack unwinds. Undo
+                    // the registration first, or a caller that catches the error
+                    // leaves this group holding a pointer to a dead object.
+                    try
+                    {
+                        SignalDetectDuplicates();
+                    }
+                    catch (...)
+                    {
+                        children.pop_back();
+                        throw;
+                    }
+#else
                     SignalDetectDuplicates();
+#endif
                 }
             }
 
@@ -3392,8 +3409,11 @@ namespace args
 
             void AddCompletion(CompletionFlag &completionFlag)
             {
-                completion = &completionFlag;
+                // Only take the pointer once registration has succeeded: Add()
+                // throws on a duplicate flag, and the flag it was handed is
+                // gone by the time that error reaches the caller.
                 Add(completionFlag);
+                completion = &completionFlag;
             }
 
             /** The program name for help generation

meson.build

@@ -36,6 +36,7 @@ test_names = [
   'count_flag',
   'custom_types',
   'default_values',
+  'duplicate_flag_rollback',
   'extra_positionals',
   'get_assignable',
   'get_program_line',

test/duplicate_flag_rollback.cxx

@@ -0,0 +1,74 @@
+/* Copyright (c) Taylor Richberger <taylor@axfive.net>
+ * This code is released under the license described in the LICENSE file
+ */
+
+#include "test_common.hxx"
+
+#include <args.hxx>
+
+#include "test_helpers.hxx"
+
+void testFailedFlagIsNotRegistered()
+{
+    args::ArgumentParser parser("This is a test program.");
+    args::Flag flag_a(parser, "aone", "test flag", {'a', "aone"});
+
+    const auto children = parser.Children().size();
+    test::require_throws_as<args::ParseError>([&]{
+        args::Flag flag_b(parser, "atwo", "test flag", {'a', "atwo"});
+    });
+    test::require(parser.Children().size() == children);
+}
+
+void testFailedFlagInGroupIsNotRegistered()
+{
+    args::ArgumentParser parser("This is a test program.");
+    args::Group group(parser, "This is a test group.", args::Group::Validators::DontCare);
+    args::Flag flag_a(group, "aone", "test flag", {'a', "aone"});
+
+    const auto children = group.Children().size();
+    test::require_throws_as<args::ParseError>([&]{
+        args::Flag flag_b(group, "atwo", "test flag", {'a', "atwo"});
+    });
+    test::require(group.Children().size() == children);
+}
+
+void testParserStillUsableAfterCaughtDuplicate()
+{
+    args::ArgumentParser parser("This is a test program.");
+    args::Flag flag_a(parser, "aone", "test flag", {'a', "aone"});
+    args::Positional<std::string> pos(parser, "pos", "test positional");
+
+    test::require_throws_as<args::ParseError>([&]{
+        args::Flag flag_b(parser, "atwo", "test flag", {'a', "atwo"});
+    });
+
+    test::require_nothrow([&]{ parser.ParseArgs(std::vector<std::string>{"-a", "value"}); });
+    test::require(bool(flag_a));
+    test::require(args::get(pos) == "value");
+    test::require_nothrow([&]{ (void)parser.Help(); });
+}
+
+void testFailedCompletionFlagIsNotRegistered()
+{
+    args::ArgumentParser parser("This is a test program.");
+    args::Flag flag_a(parser, "aone", "test flag", {'a', "complete"});
+
+    const auto children = parser.Children().size();
+    test::require_throws_as<args::ParseError>([&]{
+        args::CompletionFlag completion(parser, {"complete"});
+    });
+    test::require(parser.Children().size() == children);
+
+    // The parser must not be left pointing at the completion flag it rejected.
+    test::require_nothrow([&]{ parser.ParseArgs(std::vector<std::string>{"--complete"}); });
+    test::require(bool(flag_a));
+}
+
+int main()
+{
+    testFailedFlagIsNotRegistered();
+    testFailedFlagInGroupIsNotRegistered();
+    testParserStillUsableAfterCaughtDuplicate();
+    testFailedCompletionFlagIsNotRegistered();
+}