From ed48bfe69e01f132e4355c09c165ba9ada53865b Mon Sep 17 00:00:00 2001 From: Sayed Kaif Date: Mon, 3 Aug 2026 18:39:04 +0530 Subject: [PATCH] roll back Group::Add when duplicate flag detection throws --- args.hxx | 22 +++++++++- meson.build | 1 + test/duplicate_flag_rollback.cxx | 74 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 test/duplicate_flag_rollback.cxx diff --git a/args.hxx b/args.hxx index a2d6cf7..ba25c26 100644 --- a/args.hxx +++ b/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 diff --git a/meson.build b/meson.build index 235d764..4736173 100644 --- a/meson.build +++ b/meson.build @@ -36,6 +36,7 @@ test_names = [ 'count_flag', 'custom_types', 'default_values', + 'duplicate_flag_rollback', 'extra_positionals', 'get_assignable', 'get_program_line', diff --git a/test/duplicate_flag_rollback.cxx b/test/duplicate_flag_rollback.cxx new file mode 100644 index 0000000..def8220 --- /dev/null +++ b/test/duplicate_flag_rollback.cxx @@ -0,0 +1,74 @@ +/* Copyright (c) Taylor Richberger + * This code is released under the license described in the LICENSE file + */ + +#include "test_common.hxx" + +#include + +#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::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::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 pos(parser, "pos", "test positional"); + + test::require_throws_as([&]{ + args::Flag flag_b(parser, "atwo", "test flag", {'a', "atwo"}); + }); + + test::require_nothrow([&]{ parser.ParseArgs(std::vector{"-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::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{"--complete"}); }); + test::require(bool(flag_a)); +} + +int main() +{ + testFailedFlagIsNotRegistered(); + testFailedFlagInGroupIsNotRegistered(); + testParserStillUsableAfterCaughtDuplicate(); + testFailedCompletionFlagIsNotRegistered(); +}