{"thread":{"id":"66227","subject":"[PATCH 0/2] die_for_incompatible_opts(): unbounded number of options","startedAt":"2026-08-26T23:31:55Z","lastAt":"2026-08-30T20:55:34Z","messageCount":13,"participants":["Junio C Hamano","Elijah Newren","Jeff King","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"551323","messageId":"20260826233152.1703497-1-gitster@pobox.com","threadId":"66227","inReplyTo":null,"subject":"[PATCH 0/2] die_for_incompatible_opts(): unbounded number of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T23:31:50Z","receivedAt":"2026-08-26T23:31:55Z","isPatch":true,"body":"We have die_for_incompatible_optN() (for 2 <= N <= 4) to check and\ncomplain when two or more among N mutually incompatible options are\nused.\n\nWhat should a developer do if there are more than four options that\ncannot be used at once?\n\nIntroduce die_for_incompatible_opts(), which can handle an arbitrary\nnumber of mutually exclusive options.  This is done in two steps:\n\n - The API for existing functions takes N pairs (for 2 <= N <= 4) of\n   'int set, const char *name' parameters that signal which options\n   are set.  This parameter order is inconvenient for varargs, where\n   a sentinel value marks the end of the argument list (and there is\n   no clear sentinel value of type 'int').  The first patch rewrites\n   all implementations and callers of die_for_incompatible_optN() to\n   swap the parameter order to pairs of 'const char *name, int set'.\n\n - The second patch then introduces die_for_incompatible_opts(),\n   which takes an arbitrary number of 'const char *name, int set'\n   pairs terminated by a NULL sentinel.\n\nWe could do without the first step and use the 'const char *, int'\norder only in die_for_incompatible_opts(), leaving the traditional\ndie_for_incompatible_opt[234]() functions using the\n'int, const char *' order, but using a consistent ordering is\nlikely easier in the long run.\n\n 1/2: die_for_incompatible_optN: swap the order of arguments\n 2/2: die_for_incompatible_opts(): accept more than four options\n\n builtin/add.c          |  6 +++---\n builtin/clone.c        |  8 ++++----\n builtin/commit.c       | 24 ++++++++++++------------\n builtin/difftool.c     |  6 +++---\n builtin/gc.c           |  8 ++++----\n builtin/grep.c         |  6 +++---\n builtin/log.c          |  6 +++---\n builtin/merge-tree.c   |  8 ++++----\n builtin/pack-objects.c | 21 ++++++++++-----------\n builtin/push.c         |  8 ++++----\n builtin/repack.c       | 12 +++++++-----\n builtin/replay.c       | 18 +++++++++---------\n builtin/rev-list.c     |  6 +++---\n builtin/show-ref.c     |  7 ++++---\n parse-options.c        | 26 ++++++++++++++------------\n parse-options.h        | 40 +++++++++++++++++++++++-----------------\n revision.c             | 26 +++++++++++++-------------\n 17 files changed, 123 insertions(+), 113 deletions(-)\n\n-- \n2.55.0-862-g3c6f97f7b9\n\n"},{"id":"551324","messageId":"20260826233152.1703497-2-gitster@pobox.com","threadId":"66227","inReplyTo":"20260826233152.1703497-1-gitster@pobox.com","subject":"[PATCH 1/2] die_for_incompatible_optN: swap the order of arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T23:31:51Z","receivedAt":"2026-08-26T23:31:56Z","isPatch":true,"body":"die_for_incompatible_opt<N>() takes N pairs of <int set, const char *name>\nand complains if there is two or more pairs whose \"set\" part is non-zero.\n\nTo implement a vararg die_for_incompatible_opts() to supersede them,\nhowever, having an integer parameter as the first of the pair is a\nbit inconvenient sentinel.  Swap the order of these pairs so that\nthe \"const char *name\" comes first and then \"int set_or_unset\" comes\nnext.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/add.c          |  6 +++---\n builtin/clone.c        |  8 ++++----\n builtin/commit.c       | 24 ++++++++++++------------\n builtin/difftool.c     |  6 +++---\n builtin/gc.c           |  8 ++++----\n builtin/grep.c         |  6 +++---\n builtin/log.c          |  6 +++---\n builtin/merge-tree.c   |  8 ++++----\n builtin/pack-objects.c | 21 ++++++++++-----------\n builtin/push.c         |  8 ++++----\n builtin/repack.c       | 12 +++++++-----\n builtin/replay.c       | 18 +++++++++---------\n builtin/rev-list.c     |  6 +++---\n builtin/show-ref.c     |  7 ++++---\n parse-options.c        |  8 ++++----\n parse-options.h        | 33 ++++++++++++++++-----------------\n revision.c             | 26 +++++++++++++-------------\n 17 files changed, 106 insertions(+), 105 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex eab8f03cad..c9cb4e8265 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -511,9 +511,9 @@ int cmd_add(int argc,\n \telse if (take_worktree_changes && ADDREMOVE_DEFAULT)\n \t\taddremove = 0; /* \"-u\" was given but not \"-A\" */\n \n-\tdie_for_incompatible_opt3(take_worktree_changes, \"-u/--update\",\n-\t\t\t\t  0 < addremove_explicit, \"-A/--all\",\n-\t\t\t\t  add_resolved, \"--resolved\");\n+\tdie_for_incompatible_opt3(\"-u/--update\", take_worktree_changes,\n+\t\t\t\t  \"-A/--all\", 0 < addremove_explicit,\n+\t\t\t\t  \"--resolved\", add_resolved);\n \n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"--ignore-missing\", \"--dry-run\");\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 5b25cca510..7ebf6c31e2 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1361,10 +1361,10 @@ int cmd_clone(int argc,\n \n \ttransport_set_option(transport, TRANS_OPT_KEEP, \"yes\");\n \n-\tdie_for_incompatible_opt2(!!option_rev, \"--revision\",\n-\t\t\t\t  !!option_branch, \"--branch\");\n-\tdie_for_incompatible_opt2(!!option_rev, \"--revision\",\n-\t\t\t\t  option_mirror, \"--mirror\");\n+\tdie_for_incompatible_opt2(\"--revision\", !!option_rev,\n+\t\t\t\t  \"--branch\", !!option_branch);\n+\tdie_for_incompatible_opt2(\"--revision\", !!option_rev,\n+\t\t\t\t  \"--mirror\", option_mirror);\n \n \tif (reject_shallow)\n \t\ttransport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, \"1\");\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 28f6174503..31c58491aa 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1338,14 +1338,14 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t}\n \tif (fixup_message && squash_message)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--squash\", \"--fixup\");\n-\tdie_for_incompatible_opt4(!!use_message, \"-C\",\n-\t\t\t\t  !!edit_message, \"-c\",\n-\t\t\t\t  !!logfile, \"-F\",\n-\t\t\t\t  !!fixup_message, \"--fixup\");\n-\tdie_for_incompatible_opt4(have_option_m, \"-m\",\n-\t\t\t\t  !!edit_message, \"-c\",\n-\t\t\t\t  !!use_message, \"-C\",\n-\t\t\t\t  !!logfile, \"-F\");\n+\tdie_for_incompatible_opt4(\"-C\", !!use_message,\n+\t\t\t\t  \"-c\", !!edit_message,\n+\t\t\t\t  \"-F\", !!logfile,\n+\t\t\t\t  \"--fixup\", !!fixup_message);\n+\tdie_for_incompatible_opt4(\"-m\", have_option_m,\n+\t\t\t\t  \"-c\", !!edit_message,\n+\t\t\t\t  \"-C\", !!use_message,\n+\t\t\t\t  \"-F\", !!logfile);\n \tif (use_message || edit_message || logfile ||fixup_message || have_option_m)\n \t\tFREE_AND_NULL(template_file);\n \tif (edit_message)\n@@ -1371,10 +1371,10 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (patch_interactive)\n \t\tinteractive = 1;\n \n-\tdie_for_incompatible_opt4(also, \"-i/--include\",\n-\t\t\t\t  only, \"-o/--only\",\n-\t\t\t\t  all, \"-a/--all\",\n-\t\t\t\t  interactive, \"--interactive/-p/--patch\");\n+\tdie_for_incompatible_opt4(\"-i/--include\", also,\n+\t\t\t\t  \"-o/--only\", only,\n+\t\t\t\t  \"-a/--all\", all,\n+\t\t\t\t  \"--interactive/-p/--patch\", interactive);\n \tif (fixup_message) {\n \t\t/*\n \t\t * We limit --fixup's suboptions to only alpha characters.\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex bc7b2ea443..ecdeaa3d2c 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -773,9 +773,9 @@ int cmd_difftool(int argc,\n \t} else if (dir_diff)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--dir-diff\", \"--no-index\");\n \n-\tdie_for_incompatible_opt3(use_gui_tool == 1, \"--gui\",\n-\t\t\t\t  !!difftool_cmd, \"--tool\",\n-\t\t\t\t  !!extcmd, \"--extcmd\");\n+\tdie_for_incompatible_opt3(\"--gui\", use_gui_tool == 1,\n+\t\t\t\t  \"--tool\", !!difftool_cmd,\n+\t\t\t\t  \"--extcmd\", !!extcmd);\n \n \t/*\n \t * Explicitly specified GUI option is forwarded to git-mergetool--lib.sh;\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex de2f9e7fed..9aa69b6c6f 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1686,10 +1686,10 @@ static int maintenance_run(int argc, const char **argv, const char *prefix,\n \t\t\t     builtin_maintenance_run_usage,\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \n-\tdie_for_incompatible_opt2(opts.auto_flag, \"--auto\",\n-\t\t\t\t  opts.schedule, \"--schedule=\");\n-\tdie_for_incompatible_opt2(selected_tasks.nr, \"--task=\",\n-\t\t\t\t  opts.schedule, \"--schedule=\");\n+\tdie_for_incompatible_opt2(\"--auto\", opts.auto_flag,\n+\t\t\t\t  \"--schedule=\", opts.schedule);\n+\tdie_for_incompatible_opt2(\"--task=\", selected_tasks.nr,\n+\t\t\t\t  \"--schedule=\", opts.schedule);\n \n \tgc_config(&cfg);\n \tinitialize_task_config(&opts, &selected_tasks);\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex d3d86abe01..76b163c0da 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1396,9 +1396,9 @@ int cmd_grep(int argc,\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager(the_repository);\n \n-\tdie_for_incompatible_opt3(!use_index, \"--no-index\",\n-\t\t\t\t  untracked, \"--untracked\",\n-\t\t\t\t  cached, \"--cached\");\n+\tdie_for_incompatible_opt3(\"--no-index\", !use_index,\n+\t\t\t\t  \"--untracked\", untracked,\n+\t\t\t\t  \"--cached\", cached);\n \n \tif (!use_index || untracked) {\n \t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 350b35c556..1acc154aae 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -2247,9 +2247,9 @@ int cmd_format_patch(int argc,\n \tif (rev.show_notes)\n \t\tload_display_notes(&rev.notes_opt);\n \n-\tdie_for_incompatible_opt3(use_stdout, \"--stdout\",\n-\t\t\t\t  rev.diffopt.close_file, \"--output\",\n-\t\t\t\t  !!output_directory, \"--output-directory\");\n+\tdie_for_incompatible_opt3(\"--stdout\", use_stdout,\n+\t\t\t\t  \"--output\", rev.diffopt.close_file,\n+\t\t\t\t  \"--output-directory\", !!output_directory);\n \n \tif (use_stdout && stdout_mboxrd)\n \t\trev.commit_format = CMIT_FMT_MBOXRD;\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 49f41e520f..efd9321fc1 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -600,10 +600,10 @@ int cmd_merge_tree(int argc,\n \tif (quiet && o.show_messages == -1)\n \t\to.show_messages = 0;\n \to.merge_options.mergeability_only = quiet;\n-\tdie_for_incompatible_opt2(quiet, \"--quiet\", o.show_messages, \"--messages\");\n-\tdie_for_incompatible_opt2(quiet, \"--quiet\", o.name_only, \"--name-only\");\n-\tdie_for_incompatible_opt2(quiet, \"--quiet\", o.use_stdin, \"--stdin\");\n-\tdie_for_incompatible_opt2(quiet, \"--quiet\", !line_termination, \"-z\");\n+\tdie_for_incompatible_opt2(\"--quiet\", quiet, \"--messages\", o.show_messages);\n+\tdie_for_incompatible_opt2(\"--quiet\", quiet, \"--name-only\", o.name_only);\n+\tdie_for_incompatible_opt2(\"--quiet\", quiet, \"--stdin\", o.use_stdin);\n+\tdie_for_incompatible_opt2(\"--quiet\", quiet, \"-z\", !line_termination);\n \n \tif (xopts.nr && o.mode == MODE_TRIVIAL)\n \t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1d9dc31454..864a9ca701 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -5340,10 +5340,10 @@ int cmd_pack_objects(int argc,\n \t\tstrvec_push(&rp, \"--unpacked\");\n \t}\n \n-\tdie_for_incompatible_opt2(exclude_promisor_objects,\n-\t\t\t\t  \"--exclude-promisor-objects\",\n-\t\t\t\t  exclude_promisor_objects_best_effort,\n-\t\t\t\t  \"--exclude-promisor-objects-best-effort\");\n+\tdie_for_incompatible_opt2(\"--exclude-promisor-objects\",\n+\t\t\t\t  exclude_promisor_objects,\n+\t\t\t\t  \"--exclude-promisor-objects-best-effort\",\n+\t\t\t\t  exclude_promisor_objects_best_effort);\n \tif (exclude_promisor_objects) {\n \t\tfetch_if_missing = 0;\n \n@@ -5385,14 +5385,13 @@ int cmd_pack_objects(int argc,\n \tif (!pack_to_stdout && thin)\n \t\tdie(_(\"--thin cannot be used to build an indexable pack\"));\n \n-\tdie_for_incompatible_opt2(keep_unreachable, \"--keep-unreachable\",\n-\t\t\t\t  unpack_unreachable, \"--unpack-unreachable\");\n+\tdie_for_incompatible_opt2(\"--keep-unreachable\", keep_unreachable,\n+\t\t\t\t  \"--unpack-unreachable\", unpack_unreachable);\n \tif (!rev_list_all || !rev_list_reflog || !rev_list_index)\n \t\tunpack_unreachable_expiration = 0;\n \n-\tdie_for_incompatible_opt2(stdin_packs, \"--stdin-packs\",\n-\t\t\t\t  filter_options.choice, \"--filter\");\n-\n+\tdie_for_incompatible_opt2(\"--stdin-packs\", stdin_packs,\n+\t\t\t\t  \"--filter\", filter_options.choice);\n \n \tif (stdin_packs && use_internal_rev_list)\n \t\tdie(_(\"cannot use internal rev list with --stdin-packs\"));\n@@ -5400,8 +5399,8 @@ int cmd_pack_objects(int argc,\n \tif (cruft) {\n \t\tif (use_internal_rev_list)\n \t\t\tdie(_(\"cannot use internal rev list with --cruft\"));\n-\t\tdie_for_incompatible_opt2(stdin_packs, \"--stdin-packs\",\n-\t\t\t\t\t  cruft, \"--cruft\");\n+\t\tdie_for_incompatible_opt2(\"--stdin-packs\", stdin_packs,\n+\t\t\t\t\t  \"--cruft\", cruft);\n \t}\n \n \t/*\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2377b5af55..f20b2a31fe 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -753,10 +753,10 @@ int cmd_push(int argc,\n \n \trefspec_init_push(&rs, the_hash_algo);\n \n-\tdie_for_incompatible_opt4(deleterefs, \"--delete\",\n-\t\t\t\t  tags, \"--tags\",\n-\t\t\t\t  flags & TRANSPORT_PUSH_ALL, \"--all/--branches\",\n-\t\t\t\t  flags & TRANSPORT_PUSH_MIRROR, \"--mirror\");\n+\tdie_for_incompatible_opt4(\"--delete\", deleterefs,\n+\t\t\t\t  \"--tags\", tags,\n+\t\t\t\t  \"--all/--branches\", flags & TRANSPORT_PUSH_ALL,\n+\t\t\t\t  \"--mirror\", flags & TRANSPORT_PUSH_MIRROR);\n \tif (deleterefs && argc < 2)\n \t\tdie(_(\"--delete doesn't make sense without any refs\"));\n \ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex c4360382c1..bdac72a562 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -282,8 +282,8 @@ int cmd_repack(int argc,\n \tpo_args.depth = xstrdup_or_null(opt_depth);\n \tpo_args.threads = xstrdup_or_null(opt_threads);\n \n-\tdie_for_incompatible_opt2(drop_filtered, \"--drop-filtered\",\n-\t\t!!filter_to, \"--filter-to\");\n+\tdie_for_incompatible_opt2(\"--drop-filtered\", drop_filtered,\n+\t\t\t\t  \"--filter-to\", !!filter_to);\n \n \tif (dry_run && !drop_filtered)\n \t\tdie(_(\"--dry-run only takes effect with --drop-filtered\"));\n@@ -397,9 +397,11 @@ int cmd_repack(int argc,\n \tif (delete_redundant && repo->repository_format_precious_objects)\n \t\tdie(_(\"cannot delete packs in a precious-objects repo\"));\n \n-\tdie_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), \"-A\",\n-\t\t\t\t  keep_unreachable, \"-k/--keep-unreachable\",\n-\t\t\t\t  pack_everything & PACK_CRUFT, \"--cruft\");\n+\tdie_for_incompatible_opt3(\"-A\",\n+\t\t\t\t  unpack_unreachable ||\n+\t\t\t\t  (pack_everything & LOOSEN_UNREACHABLE),\n+\t\t\t\t  \"-k/--keep-unreachable\", keep_unreachable,\n+\t\t\t\t  \"--cruft\", pack_everything & PACK_CRUFT);\n \n \tif (pack_everything & PACK_CRUFT)\n \t\tpack_everything |= ALL_INTO_ONE;\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 39e3a86f6c..b97d9de21f 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -123,15 +123,15 @@ int cmd_replay(int argc,\n \t\tusage_with_options(replay_usage, replay_options);\n \t}\n \n-\tdie_for_incompatible_opt3(!!opts.onto, \"--onto\",\n-\t\t\t\t  !!opts.advance, \"--advance\",\n-\t\t\t\t  !!opts.revert, \"--revert\");\n-\tdie_for_incompatible_opt2(!!opts.advance, \"--advance\",\n-\t\t\t\t  opts.contained, \"--contained\");\n-\tdie_for_incompatible_opt2(!!opts.revert, \"--revert\",\n-\t\t\t\t  opts.contained, \"--contained\");\n-\tdie_for_incompatible_opt2(!!opts.ref, \"--ref\",\n-\t\t\t\t  !!opts.contained, \"--contained\");\n+\tdie_for_incompatible_opt3(\"--onto\", !!opts.onto,\n+\t\t\t\t  \"--advance\", !!opts.advance,\n+\t\t\t\t  \"--revert\", !!opts.revert);\n+\tdie_for_incompatible_opt2(\"--advance\", !!opts.advance,\n+\t\t\t\t  \"--contained\", opts.contained);\n+\tdie_for_incompatible_opt2(\"--revert\", !!opts.revert,\n+\t\t\t\t  \"--contained\", opts.contained);\n+\tdie_for_incompatible_opt2(\"--ref\", !!opts.ref,\n+\t\t\t\t  \"--contained\", !!opts.contained);\n \n \t/* Parse ref action mode from command line or config */\n \tref_mode = get_ref_action_mode(repo, ref_action);\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 02818b81c6..8e962b09da 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -755,9 +755,9 @@ int cmd_rev_list(int argc,\n \t\t}\n \t}\n \n-\tdie_for_incompatible_opt2(revs.exclude_promisor_objects,\n-\t\t\t\t  \"--exclude_promisor_objects\",\n-\t\t\t\t  arg_missing_action, \"--missing\");\n+\tdie_for_incompatible_opt2(\"--exclude_promisor_objects\",\n+\t\t\t\t  revs.exclude_promisor_objects,\n+\t\t\t\t  \"--missing\", arg_missing_action);\n \n \tif (arg_missing_action)\n \t\trevs.do_not_die_on_missing_objects = 1;\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex d508441632..8f942ecbfc 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -337,9 +337,10 @@ struct repository *repo UNUSED)\n \targc = parse_options(argc, argv, prefix, show_ref_options,\n \t\t\t     show_ref_usage, 0);\n \n-\tdie_for_incompatible_opt3(exclude_existing_opts.enabled, \"--exclude-existing\",\n-\t\t\t\t  verify, \"--verify\",\n-\t\t\t\t  exists, \"--exists\");\n+\tdie_for_incompatible_opt3(\"--exclude-existing\",\n+\t\t\t\t  exclude_existing_opts.enabled,\n+\t\t\t\t  \"--verify\", verify,\n+\t\t\t\t  \"--exists\", exists);\n \n \tif (exclude_existing_opts.enabled)\n \t\treturn cmd_show_ref__exclude_existing(&exclude_existing_opts);\ndiff --git a/parse-options.c b/parse-options.c\nindex 4519ead9dc..b56bc7e419 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1535,10 +1535,10 @@ void NORETURN usage_msg_optf(const char * const fmt,\n \tusage_msg_opt(msg.buf, usagestr, options);\n }\n \n-void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n-\t\t\t       int opt2, const char *opt2_name,\n-\t\t\t       int opt3, const char *opt3_name,\n-\t\t\t       int opt4, const char *opt4_name)\n+void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n+\t\t\t       const char *opt2_name, int opt2,\n+\t\t\t       const char *opt3_name, int opt3,\n+\t\t\t       const char *opt4_name, int opt4)\n {\n \tint count = 0;\n \tconst char *options[4];\ndiff --git a/parse-options.h b/parse-options.h\nindex d7f896a933..888949ab61 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -441,29 +441,28 @@ void NORETURN usage_msg_optf(const char *fmt,\n \t\t\t     const char * const *usagestr,\n \t\t\t     const struct option *options, ...);\n \n-void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n-\t\t\t       int opt2, const char *opt2_name,\n-\t\t\t       int opt3, const char *opt3_name,\n-\t\t\t       int opt4, const char *opt4_name);\n+void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n+\t\t\t       const char *opt2_name, int opt2,\n+\t\t\t       const char *opt3_name, int opt3,\n+\t\t\t       const char *opt4_name, int opt4);\n \n \n-static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,\n-\t\t\t\t\t     int opt2, const char *opt2_name,\n-\t\t\t\t\t     int opt3, const char *opt3_name)\n+static inline void die_for_incompatible_opt3(const char *opt1_name, int opt1,\n+\t\t\t\t\t     const char *opt2_name, int opt2,\n+\t\t\t\t\t     const char *opt3_name, int opt3)\n {\n-\tdie_for_incompatible_opt4(opt1, opt1_name,\n-\t\t\t\t  opt2, opt2_name,\n-\t\t\t\t  opt3, opt3_name,\n-\t\t\t\t  0, \"\");\n+\tdie_for_incompatible_opt4(opt1_name, opt1,\n+\t\t\t\t  opt2_name, opt2,\n+\t\t\t\t  opt3_name, opt3,\n+\t\t\t\t  \"\", 0);\n }\n \n-static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,\n-\t\t\t\t\t     int opt2, const char *opt2_name)\n+static inline void die_for_incompatible_opt2(const char *opt1_name, int opt1,\n+\t\t\t\t\t     const char *opt2_name, int opt2)\n {\n-\tdie_for_incompatible_opt4(opt1, opt1_name,\n-\t\t\t\t  opt2, opt2_name,\n-\t\t\t\t  0, \"\",\n-\t\t\t\t  0, \"\");\n+\tdie_for_incompatible_opt4(opt1_name, opt1,\n+\t\t\t\t  opt2_name, opt2,\n+\t\t\t\t  \"\", 0, \"\", 0);\n }\n \n /*\ndiff --git a/revision.c b/revision.c\nindex 50dc8b1991..b6108501fd 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2352,27 +2352,27 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n \t\tif (revs->max_count_type == 1)\n-\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n-\t\t\t\t\t\t  \"--max-count-oldest\");\n+\t\t\tdie_for_incompatible_opt2(\"--max-count\", 1,\n+\t\t\t\t\t\t  \"--max-count-oldest\", 1);\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n \t\trevs->max_count_type = 0;\n \t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n \t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n-\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n-\t\t\t\t\t\t  \"--max-count-oldest\");\n+\t\t\tdie_for_incompatible_opt2(\"--max-count\", 1,\n+\t\t\t\t\t\t  \"--max-count-oldest\", 1);\n \t\tif (revs->skip_count > 0)\n-\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n-\t\t\t\t\t\t  \"--max-count-oldest\");\n+\t\t\tdie_for_incompatible_opt2(\"--skip\", 1,\n+\t\t\t\t\t\t  \"--max-count-oldest\", 1);\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n \t\trevs->max_count_type = 1;\n \t\trevs->max_count_stage = 0;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n \t\tif (revs->max_count_type == 1)\n-\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n-\t\t\t\t\t\t  \"--max-count-oldest\");\n+\t\t\tdie_for_incompatible_opt2(\"--skip\", 1,\n+\t\t\t\t\t\t  \"--max-count-oldest\", 1);\n \t\trevs->skip_count = parse_count(optarg);\n \t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -3205,12 +3205,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t/*\n \t * Limitations on the graph functionality\n \t */\n-\tdie_for_incompatible_opt3(!!revs->graph, \"--graph\",\n-\t\t\t\t  !!revs->reverse, \"--reverse\",\n-\t\t\t\t  !!revs->reflog_info, \"--walk-reflogs\");\n+\tdie_for_incompatible_opt3(\"--graph\", !!revs->graph,\n+\t\t\t\t  \"--reverse\", !!revs->reverse,\n+\t\t\t\t  \"--walk-reflogs\", !!revs->reflog_info);\n \n-\tdie_for_incompatible_opt2(!!revs->boundary, \"--boundary\",\n-\t\t\t\t  !!revs->maximal_only, \"--maximal-only\");\n+\tdie_for_incompatible_opt2(\"--boundary\", !!revs->boundary,\n+\t\t\t\t  \"--maximal-only\", !!revs->maximal_only);\n \n \tif (revs->no_walk && revs->graph)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--no-walk\", \"--graph\");\n-- \n2.55.0-862-g3c6f97f7b9\n\n"},{"id":"551325","messageId":"20260826233152.1703497-3-gitster@pobox.com","threadId":"66227","inReplyTo":"20260826233152.1703497-1-gitster@pobox.com","subject":"[PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T23:31:52Z","receivedAt":"2026-08-26T23:31:58Z","isPatch":true,"body":"Introduce die_for_incompatible_opts(), which takes an arbitrary\nand unbounded number of <option-name, option-set> pairs and\ncomplains when two or more of these options are set at the same time.\n\nReimplement die_for_incompatible_opt4() and others in terms of this\nfunction.\n\nTo avoid allocation costs, the implementation reports only the first\nfour mutually incompatible options used.\n\nThis behavior is deliberate.  If a set of ten options were mutually\nexclusive and a user specified seven of them at once, they would be\ntold that the first four cannot be used together.  If the user then\ntries the remaining three, the same error for the remaining three\nwould be reported.  It is dubious that there is any practical\ndownside to not reporting all seven incompatible options at once,\nespecially given that there are other three mutually incompatible\noptions that the user will not be told about with this message\nanyway.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n parse-options.c | 26 ++++++++++++++------------\n parse-options.h | 15 +++++++++++----\n 2 files changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex b56bc7e419..163842837c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1535,26 +1535,28 @@ void NORETURN usage_msg_optf(const char * const fmt,\n \tusage_msg_opt(msg.buf, usagestr, options);\n }\n \n-void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n-\t\t\t       const char *opt2_name, int opt2,\n-\t\t\t       const char *opt3_name, int opt3,\n-\t\t\t       const char *opt4_name, int opt4)\n+void die_for_incompatible_opts(const char *opt1_name, int opt1, ...)\n {\n-\tint count = 0;\n+\tunsigned count = 0;\n \tconst char *options[4];\n+\tva_list ap;\n+\n+\tva_start(ap, opt1);\n \n \tif (opt1)\n \t\toptions[count++] = opt1_name;\n-\tif (opt2)\n-\t\toptions[count++] = opt2_name;\n-\tif (opt3)\n-\t\toptions[count++] = opt3_name;\n-\tif (opt4)\n-\t\toptions[count++] = opt4_name;\n+\twhile (count < ARRAY_SIZE(options)) {\n+\t\tconst char *name = va_arg(ap, const char *);\n+\t\tif (!name)\n+\t\t\tbreak;\n+\t\tif (va_arg(ap, int))\n+\t\t\toptions[count++] = name;\n+\t}\n+\n \tswitch (count) {\n \tcase 4:\n \t\tdie(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n-\t\t    opt1_name, opt2_name, opt3_name, opt4_name);\n+\t\t    options[0], options[1], options[2], options[3]);\n \t\tbreak;\n \tcase 3:\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\ndiff --git a/parse-options.h b/parse-options.h\nindex 888949ab61..79e4de9b32 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -441,11 +441,18 @@ void NORETURN usage_msg_optf(const char *fmt,\n \t\t\t     const char * const *usagestr,\n \t\t\t     const struct option *options, ...);\n \n-void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n-\t\t\t       const char *opt2_name, int opt2,\n-\t\t\t       const char *opt3_name, int opt3,\n-\t\t\t       const char *opt4_name, int opt4);\n+void die_for_incompatible_opts(const char *opt1_name, int opt1, ...);\n \n+static inline void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n+\t\t\t\t\t     const char *opt2_name, int opt2,\n+\t\t\t\t\t     const char *opt3_name, int opt3,\n+\t\t\t\t\t     const char *opt4_name, int opt4)\n+{\n+\tdie_for_incompatible_opts(opt1_name, opt1,\n+\t\t\t\t  opt2_name, opt2,\n+\t\t\t\t  opt3_name, opt3,\n+\t\t\t\t  opt4_name, opt4, NULL);\n+}\n \n static inline void die_for_incompatible_opt3(const char *opt1_name, int opt1,\n \t\t\t\t\t     const char *opt2_name, int opt2,\n-- \n2.55.0-862-g3c6f97f7b9\n\n"},{"id":"551334","messageId":"CABPp-BG2PJ7AyC2ctPuX0bmkFd_cGmNz+XtbjdjCbMrH4_d99A@mail.gmail.com","threadId":"66227","inReplyTo":"20260826233152.1703497-3-gitster@pobox.com","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-27T01:19:22Z","receivedAt":"2026-08-27T01:19:35Z","isPatch":true,"body":"On Wed, Aug 26, 2026 at 4:32 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n[...]\n> +void die_for_incompatible_opts(const char *opt1_name, int opt1, ...)\n>  {\n> -       int count = 0;\n> +       unsigned count = 0;\n>         const char *options[4];\n> +       va_list ap;\n> +\n> +       va_start(ap, opt1);\n>\n>         if (opt1)\n>                 options[count++] = opt1_name;\n> -       if (opt2)\n> -               options[count++] = opt2_name;\n> -       if (opt3)\n> -               options[count++] = opt3_name;\n> -       if (opt4)\n> -               options[count++] = opt4_name;\n> +       while (count < ARRAY_SIZE(options)) {\n> +               const char *name = va_arg(ap, const char *);\n> +               if (!name)\n> +                       break;\n> +               if (va_arg(ap, int))\n> +                       options[count++] = name;\n> +       }\n> +\n>         switch (count) {\n>         case 4:\n>                 die(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n> -                   opt1_name, opt2_name, opt3_name, opt4_name);\n> +                   options[0], options[1], options[2], options[3]);\n>                 break;\n>         case 3:\n>                 die(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n> diff --git a/parse-options.h b/parse-options.h\n\nva_start() without a va_end()?\n"},{"id":"551336","messageId":"20260827045515.GA176544@coredump.intra.peff.net","threadId":"66227","inReplyTo":"20260826233152.1703497-3-gitster@pobox.com","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-27T04:55:15Z","receivedAt":"2026-08-27T04:55:22Z","isPatch":true,"body":"On Wed, Aug 26, 2026 at 04:31:52PM -0700, Junio C Hamano wrote:\n\n> To avoid allocation costs, the implementation reports only the first\n> four mutually incompatible options used.\n> \n> This behavior is deliberate.  If a set of ten options were mutually\n> exclusive and a user specified seven of them at once, they would be\n> told that the first four cannot be used together.  If the user then\n> tries the remaining three, the same error for the remaining three\n> would be reported.  It is dubious that there is any practical\n> downside to not reporting all seven incompatible options at once,\n> especially given that there are other three mutually incompatible\n> options that the user will not be told about with this message\n> anyway.\n\nIt took me a minute to understand why we would even want to have an\narbitrary-sized input if we are capping at 4 anyway. The answer is that\nwe are capping at 4 options _that the user actually specified_. But the\ninput can be the total set of conflicting options, which is greater. OK.\n\nReally we could cap at 2 if we wanted to be technically correct, but it\nmight annoy the user to find each pair iteratively.\n\nSo that makes sense. Of course the follow-on question is whether any\ncallers actually want to pass more than 4 options. I don't see any\npatches adding new calls.\n\n> -void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n> -\t\t\t       const char *opt2_name, int opt2,\n> -\t\t\t       const char *opt3_name, int opt3,\n> -\t\t\t       const char *opt4_name, int opt4)\n\nOne nice thing about foo4() without varargs is that the compiler will\ntell you if you messed it up. The obvious downside being that you have\nto count in order to avoid messing it up. ;)\n\nBut now we can forget the NULL terminator and cause a runtime problem.\nSo we probably want LAST_ARG_MUST_BE_NULL in the header file here:\n\n> +void die_for_incompatible_opts(const char *opt1_name, int opt1, ...);\n\nThe rest of the patch looks OK, but just a few observations.\n\n> +void die_for_incompatible_opts(const char *opt1_name, int opt1, ...)\n>  {\n> -\tint count = 0;\n> +\tunsigned count = 0;\n>  \tconst char *options[4];\n> +\tva_list ap;\n> +\n> +\tva_start(ap, opt1);\n>  \n>  \tif (opt1)\n>  \t\toptions[count++] = opt1_name;\n> -\tif (opt2)\n> -\t\toptions[count++] = opt2_name;\n> -\tif (opt3)\n> -\t\toptions[count++] = opt3_name;\n> -\tif (opt4)\n> -\t\toptions[count++] = opt4_name;\n> +\twhile (count < ARRAY_SIZE(options)) {\n\nUsing ARRAY_SIZE() is nice, because we could in theory bump this 4\nlater. Though sadly here:\n\n>  \tswitch (count) {\n>  \tcase 4:\n>  \t\tdie(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n> -\t\t    opt1_name, opt2_name, opt3_name, opt4_name);\n> +\t\t    options[0], options[1], options[2], options[3]);\n\nwe still hard-code various count values. It probably would be fine to\nallocate a buffer for the message, though I guess that pushes\ntranslators into lego-land.\n\n> +static inline void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n> +\t\t\t\t\t     const char *opt2_name, int opt2,\n> +\t\t\t\t\t     const char *opt3_name, int opt3,\n> +\t\t\t\t\t     const char *opt4_name, int opt4)\n> +{\n> +\tdie_for_incompatible_opts(opt1_name, opt1,\n> +\t\t\t\t  opt2_name, opt2,\n> +\t\t\t\t  opt3_name, opt3,\n> +\t\t\t\t  opt4_name, opt4, NULL);\n> +}\n\nOK, now we wrap the arbitrary-sized version. The \"3\" and \"2\" variants\ncould probably be cleaned up slightly by calling it, too, rather than\npassing dummy 0/\"\" values.\n\n-Peff\n"},{"id":"551357","messageId":"xmqq4igfd4nl.fsf@gitster.g","threadId":"66227","inReplyTo":"CABPp-BG2PJ7AyC2ctPuX0bmkFd_cGmNz+XtbjdjCbMrH4_d99A@mail.gmail.com","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-27T14:22:38Z","receivedAt":"2026-08-27T14:22:41Z","isPatch":true,"body":"Elijah Newren <newren@gmail.com> writes:\n\n>> diff --git a/parse-options.h b/parse-options.h\n>\n> va_start() without a va_end()?\n\nGood eyes.  Thanks.\n"},{"id":"551358","messageId":"xmqqv78vbphh.fsf@gitster.g","threadId":"66227","inReplyTo":"20260827045515.GA176544@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-27T14:35:38Z","receivedAt":"2026-08-27T14:35:41Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> It took me a minute to understand why we would even want to have an\n> arbitrary-sized input if we are capping at 4 anyway. The answer is that\n> we are capping at 4 options _that the user actually specified_. But the\n> input can be the total set of conflicting options, which is greater. OK.\n>\n> Really we could cap at 2 if we wanted to be technically correct, but it\n> might annoy the user to find each pair iteratively.\n>\n> So that makes sense. Of course the follow-on question is whether any\n> callers actually want to pass more than 4 options. I don't see any\n> patches adding new calls.\n\nThere isn't.  While I was writing [*], I wondered if the two calls\nnext to each other for opt3 and opt4 want to be combined to opt7.\n\n  *  https://lore.kernel.org/git/xmqq1pbkefh0.fsf@gitster.g/\n\n\n\n>> -void die_for_incompatible_opt4(const char *opt1_name, int opt1,\n>> -\t\t\t       const char *opt2_name, int opt2,\n>> -\t\t\t       const char *opt3_name, int opt3,\n>> -\t\t\t       const char *opt4_name, int opt4)\n>\n> One nice thing about foo4() without varargs is that the compiler will\n> tell you if you messed it up. The obvious downside being that you have\n> to count in order to avoid messing it up. ;)\n\nYes.  I like that and that is why the static inlines are kept to\ncover the most common cases.\n\nI think I can do without [1/2], by the way.\n\n - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N\n   pairs of <int, const char *>\n\n - die_for_incompatible_opts() will take pairs of <int, const char *>,\n   expects \"int\" to be 0 (not set), 1 (set), or EOF==-1 (sentinel).\n\n - static inline void die_for_incompatible_opt2() emulation layer\n   will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2,\n   opt2_name, EOF).  Similarly for opt3() and opt4() variants.\n\n> Using ARRAY_SIZE() is nice, because we could in theory bump this 4\n> later. Though sadly here:\n>\n>>  \tswitch (count) {\n>>  \tcase 4:\n>>  \t\tdie(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n>> -\t\t    opt1_name, opt2_name, opt3_name, opt4_name);\n>> +\t\t    options[0], options[1], options[2], options[3]);\n>\n> we still hard-code various count values. It probably would be fine to\n> allocate a buffer for the message, though I guess that pushes\n> translators into lego-land.\n\nVery true.\n\nWe could switch to dynamic allocations immediately after we see\noption[] filled, as we are committed to die() at that point and can\nafford to waste cycles.  That way, for die_for_incompatible_opt10()\nwhen the end-user uses 7 of them, we can fill option[4], switch to\ndynamic allocation to collect all 7 of them and report.\n\nThe reason I chose not to is primarily because we cannot use the\nexisting message templates in that case, hurting i18n/l10n.\n"},{"id":"551380","messageId":"xmqqbjana2wv.fsf@gitster.g","threadId":"66227","inReplyTo":"20260826233152.1703497-1-gitster@pobox.com","subject":"[PATCH v2] die_for_incompatible_opts(): unbounded number of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-27T17:28:32Z","receivedAt":"2026-08-27T17:28:35Z","isPatch":true,"body":"We have die_for_incompatible_optN() (for 2 <= N <= 4) to check and\ncomplain when two or more among N mutually incompatible options are\nused.\n\nWhat should a developer do if there are more than four options that\ncannot be used at once?\n\nIntroduce die_for_incompatible_opts(), which can handle an arbitrary\nnumber of mutually exclusive options, and rewrite existing variants\nusing it.\n\nThe new function takes N pairs of <bool optN, const char *nameN>,\nfollowed by EOF.  Note that even if the caller passes bool, it is\npromoted to platform-natural int when calling this variadic\nfunction.  Thus, the implementation uses va_arg(ap, int) to extract\nthe value, which allows it to distinguish between bool and EOF\nserving as the sentinel.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n parse-options.c | 29 +++++++++++++++++------------\n parse-options.h | 38 ++++++++++++++++++++++++++------------\n 2 files changed, 43 insertions(+), 24 deletions(-)\n\ndiff --git c/parse-options.c w/parse-options.c\nindex 4519ead9dc..0aad1e5373 100644\n--- c/parse-options.c\n+++ w/parse-options.c\n@@ -1535,26 +1535,31 @@ void NORETURN usage_msg_optf(const char * const fmt,\n \tusage_msg_opt(msg.buf, usagestr, options);\n }\n \n-void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n-\t\t\t       int opt2, const char *opt2_name,\n-\t\t\t       int opt3, const char *opt3_name,\n-\t\t\t       int opt4, const char *opt4_name)\n+void die_for_incompatible_opts(bool opt1, const char *opt1_name, ...)\n {\n-\tint count = 0;\n+\tunsigned count = 0;\n \tconst char *options[4];\n+\tva_list ap;\n \n \tif (opt1)\n \t\toptions[count++] = opt1_name;\n-\tif (opt2)\n-\t\toptions[count++] = opt2_name;\n-\tif (opt3)\n-\t\toptions[count++] = opt3_name;\n-\tif (opt4)\n-\t\toptions[count++] = opt4_name;\n+\tva_start(ap, opt1_name);\n+\twhile (count < ARRAY_SIZE(options)) {\n+\t\tint opt_set = va_arg(ap, int);\n+\t\tconst char *opt_name;\n+\n+\t\tif (opt_set == EOF)\n+\t\t\tbreak;\n+\t\topt_name = va_arg(ap, const char *);\n+\t\tif (opt_set)\n+\t\t\toptions[count++] = opt_name;\n+\t}\n+\tva_end(ap);\n+\n \tswitch (count) {\n \tcase 4:\n \t\tdie(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n-\t\t    opt1_name, opt2_name, opt3_name, opt4_name);\n+\t\t    options[0], options[1], options[2], options[3]);\n \t\tbreak;\n \tcase 3:\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\ndiff --git c/parse-options.h w/parse-options.h\nindex d7f896a933..50bd715b86 100644\n--- c/parse-options.h\n+++ w/parse-options.h\n@@ -441,29 +441,43 @@ void NORETURN usage_msg_optf(const char *fmt,\n \t\t\t     const char * const *usagestr,\n \t\t\t     const struct option *options, ...);\n \n-void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n-\t\t\t       int opt2, const char *opt2_name,\n-\t\t\t       int opt3, const char *opt3_name,\n-\t\t\t       int opt4, const char *opt4_name);\n+/*\n+ * Take N pairs of <bool optN, const char *opt_nameN> as parameters,\n+ * followed by EOF.  The caller declares \"The options opt_name1 through\n+ * opt_nameN exist and the command line has options whose optN is set.\"\n+ * and asks that an error be raised if two or more of these options are\n+ * set at the same time.\n+ */\n+void die_for_incompatible_opts(bool opt1, const char *opt1_name, ...);\n \n+static inline void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n+\t\t\t\t\t     int opt2, const char *opt2_name,\n+\t\t\t\t\t     int opt3, const char *opt3_name,\n+\t\t\t\t\t     int opt4, const char *opt4_name)\n+{\n+\tdie_for_incompatible_opts(!!opt1, opt1_name,\n+\t\t\t\t  !!opt2, opt2_name,\n+\t\t\t\t  !!opt3, opt3_name,\n+\t\t\t\t  !!opt4, opt4_name,\n+\t\t\t\t  EOF);\n+}\n \n static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,\n \t\t\t\t\t     int opt2, const char *opt2_name,\n \t\t\t\t\t     int opt3, const char *opt3_name)\n {\n-\tdie_for_incompatible_opt4(opt1, opt1_name,\n-\t\t\t\t  opt2, opt2_name,\n-\t\t\t\t  opt3, opt3_name,\n-\t\t\t\t  0, \"\");\n+\tdie_for_incompatible_opts(!!opt1, opt1_name,\n+\t\t\t\t  !!opt2, opt2_name,\n+\t\t\t\t  !!opt3, opt3_name,\n+\t\t\t\t  EOF);\n }\n \n static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,\n \t\t\t\t\t     int opt2, const char *opt2_name)\n {\n-\tdie_for_incompatible_opt4(opt1, opt1_name,\n-\t\t\t\t  opt2, opt2_name,\n-\t\t\t\t  0, \"\",\n-\t\t\t\t  0, \"\");\n+\tdie_for_incompatible_opts(!!opt1, opt1_name,\n+\t\t\t\t  !!opt2, opt2_name,\n+\t\t\t\t  EOF);\n }\n \n /*\n"},{"id":"551456","messageId":"20260829111418.GA40814@coredump.intra.peff.net","threadId":"66227","inReplyTo":"xmqqv78vbphh.fsf@gitster.g","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-29T11:14:18Z","receivedAt":"2026-08-29T11:14:26Z","isPatch":true,"body":"On Thu, Aug 27, 2026 at 07:35:38AM -0700, Junio C Hamano wrote:\n\n> > So that makes sense. Of course the follow-on question is whether any\n> > callers actually want to pass more than 4 options. I don't see any\n> > patches adding new calls.\n> \n> There isn't.  While I was writing [*], I wondered if the two calls\n> next to each other for opt3 and opt4 want to be combined to opt7.\n\nOK. I wonder if we're approaching churn here, but I don't have a strong\nfeeling.\n\n> I think I can do without [1/2], by the way.\n> \n>  - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N\n>    pairs of <int, const char *>\n> \n>  - die_for_incompatible_opts() will take pairs of <int, const char *>,\n>    expects \"int\" to be 0 (not set), 1 (set), or EOF==-1 (sentinel).\n> \n>  - static inline void die_for_incompatible_opt2() emulation layer\n>    will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2,\n>    opt2_name, EOF).  Similarly for opt3() and opt4() variants.\n\nYeah, but then you can't get good compiler support, since I don't think\nthere is an integer equivalent to LAST_ARG_MUST_BE_NULL. So the varargs\ninterface feels less safe (and strictly worse since we are not actually\nhelping any case that has more than 4 items).\n\nIf we're not actually exposing the varargs version and expect people to\nuse the counted wrappers, then it's not as big a risk. But then I wonder\nwhat the value of the patch is.\n\n> We could switch to dynamic allocations immediately after we see\n> option[] filled, as we are committed to die() at that point and can\n> afford to waste cycles.  That way, for die_for_incompatible_opt10()\n> when the end-user uses 7 of them, we can fill option[4], switch to\n> dynamic allocation to collect all 7 of them and report.\n> \n> The reason I chose not to is primarily because we cannot use the\n> existing message templates in that case, hurting i18n/l10n.\n\nYeah, that makes sense.\n\n-Peff\n"},{"id":"551457","messageId":"20260829111549.GB40814@coredump.intra.peff.net","threadId":"66227","inReplyTo":"xmqqbjana2wv.fsf@gitster.g","subject":"Re: [PATCH v2] die_for_incompatible_opts(): unbounded number of options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-29T11:15:49Z","receivedAt":"2026-08-29T11:15:51Z","isPatch":true,"body":"On Thu, Aug 27, 2026 at 10:28:32AM -0700, Junio C Hamano wrote:\n\n> +void die_for_incompatible_opts(bool opt1, const char *opt1_name, ...)\n\nI'm mildly negative on this, just because there's no compiler support\nfor making sure there is an EOF somewhere. Keeping patch 1 and using\nLAST_ARG_MUST_BE_NULL would be preferable, IMHO.\n\n-Peff\n"},{"id":"551467","messageId":"87a4q4u867.fsf@gitster.g","threadId":"66227","inReplyTo":"20260829111418.GA40814@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-29T17:51:28Z","receivedAt":"2026-08-29T17:51:34Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Aug 27, 2026 at 07:35:38AM -0700, Junio C Hamano wrote:\n>\n>> > So that makes sense. Of course the follow-on question is whether any\n>> > callers actually want to pass more than 4 options. I don't see any\n>> > patches adding new calls.\n>> \n>> There isn't.  While I was writing [*], I wondered if the two calls\n>> next to each other for opt3 and opt4 want to be combined to opt7.\n>\n> OK. I wonder if we're approaching churn here, but I don't have a strong\n> feeling.\n\nA quiz that I may probably fail if I were asked in a job interview:\n\n- Using die_for_incompatible_opt[234]() functions, find a way for\n  any arbitrary N (4 < N) to ensure that no more than two of N\n  options are not set at the same time.\n\n  For example, die_for_incompatible_opt5() can be written like so:\n\n    void die_for_incompatible_opt5(int opt1, const char *name1,\n\t\t\t\t   int opt2, const char *name2,\n\t\t\t\t   int opt3, const char *name3,\n\t\t\t\t   int opt4, const char *name4,\n\t\t\t\t   int opt5, const char *name5)\n    {\n\tdie_for_incompatible_opt4(opt1, name1, opt2, name2,\n\t\t\t\t  opt3, name3, opt4, name4);\n\tdie_for_incompatible_opt4(opt5, name5, opt2, name2,\n\t\t\t\t  opt3, name3, opt4, name4);\n\tdie_for_incompatible_opt2(opt5, name5, opt1, name1);\n    }\n\t\nbut can't we do better?  ;-)\n\n> Yeah, but then you can't get good compiler support, since I don't think\n> there is an integer equivalent to LAST_ARG_MUST_BE_NULL.\n\nAh, I missed that.  It certainly makes sense to flip the order of\nthese <set, name> pairs.  I suspect that nobody was thinking that\nthese eventually need to support vararg form when they first added\ndie_for_incompatible_opt2() and then later extended it to forms that\ncan support 3 and 4 options; otherwise we would certainly have\nchosen the <nameN, setN> order to allow NULL termination.\n\n"},{"id":"551468","messageId":"5d5b1f26-192f-457d-bc18-499a3d7507fa@web.de","threadId":"66227","inReplyTo":"20260829111418.GA40814@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-29T18:04:21Z","receivedAt":"2026-08-29T18:04:30Z","isPatch":true,"body":"On 8/29/26 1:14 PM, Jeff King wrote:\n> On Thu, Aug 27, 2026 at 07:35:38AM -0700, Junio C Hamano wrote:\n> \n>>> So that makes sense. Of course the follow-on question is whether any\n>>> callers actually want to pass more than 4 options. I don't see any\n>>> patches adding new calls.\n>>\n>> There isn't.  While I was writing [*], I wondered if the two calls\n>> next to each other for opt3 and opt4 want to be combined to opt7.\n> \n> OK. I wonder if we're approaching churn here, but I don't have a strong\n> feeling.\n> \n>> I think I can do without [1/2], by the way.\n>>\n>>  - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N\n>>    pairs of <int, const char *>\n>>\n>>  - die_for_incompatible_opts() will take pairs of <int, const char *>,\n>>    expects \"int\" to be 0 (not set), 1 (set), or EOF==-1 (sentinel).\n>>\n>>  - static inline void die_for_incompatible_opt2() emulation layer\n>>    will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2,\n>>    opt2_name, EOF).  Similarly for opt3() and opt4() variants.\n> \n> Yeah, but then you can't get good compiler support, since I don't think\n> there is an integer equivalent to LAST_ARG_MUST_BE_NULL. So the varargs\n> interface feels less safe (and strictly worse since we are not actually\n> helping any case that has more than 4 items).\n\nYou can still use LAST_ARG_MUST_BE_NULL if you require EOF _and_ NULL.\nLooks silly, but could be papered over with a macro:\n\n#define die_for_incompatible_opts(...) \\\n\tdie_for_incompatible_opts_internal(__VA_ARGS__, EOF, NULL)\n\nWith such a macro you don't really need LAST_ARG_MUST_BE_NULL anymore,\nthough, as it already guarantees termination by construction -- as long\nas the internal function is never called directly.\n\nIt's still less safe because it only checks the types of its first two\narguments.  On one hand this might suffice, because the rest of the\narguments just need to continue the pattern.  On the other hand it's\nerror-handling code, which tends to be tested less, so a broken\npattern might be overlooked.\n\nHere's a type-safe variant, but it looks a bit odd with all those\nmustaches:\n\nstruct used_option {\n\tconst char *name;\n\tbool used;\n};\n\n#define DIE_FOR_INCOMPATIBLE_OPTS(...) \\\n\tdie_for_incompatible_opts((struct used_option []){ \\\n\t\t__VA_ARGS__, \\\n\t\t{ NULL } \\\n\t})\n\nvoid die_for_incompatible_opts(const struct used_option *);\n\t\nstatic inline void die_for_incompatible_opt4(int opt1, const char *opt1_name,\n\t\t\t\t\t     int opt2, const char *opt2_name,\n\t\t\t\t\t     int opt3, const char *opt3_name,\n\t\t\t\t\t     int opt4, const char *opt4_name)\n{\n\tDIE_FOR_INCOMPATIBLE_OPTS({ opt1_name, opt1 },\n\t\t\t\t  { opt2_name, opt2 },\n\t\t\t\t  { opt3_name, opt3 },\n\t\t\t\t  { opt4_name, opt4 });\n}\n\n\nRené\n\n"},{"id":"551486","messageId":"xmqqfqzv1g6z.fsf@gitster.g","threadId":"66227","inReplyTo":"20260829111549.GB40814@coredump.intra.peff.net","subject":"Re: [PATCH v2] die_for_incompatible_opts(): unbounded number of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-30T20:55:32Z","receivedAt":"2026-08-30T20:55:34Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Aug 27, 2026 at 10:28:32AM -0700, Junio C Hamano wrote:\n>\n>> +void die_for_incompatible_opts(bool opt1, const char *opt1_name, ...)\n>\n> I'm mildly negative on this, just because there's no compiler support\n> for making sure there is an EOF somewhere. Keeping patch 1 and using\n> LAST_ARG_MUST_BE_NULL would be preferable, IMHO.\n\nLet's discard this topic for now.\n\nI do not like the second iteration very much, and I do not like the\n1/2 preliminary step of the first iteration even less so.\n\nThanks.\n"}]}