This PR changes the way settings are registered and retrieved to provide more compile-time safety. Currently settings are registered and retrieved like:
0// Register setting
1argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
2
3// Retrieve setting
4args.GetPathArg("-pid", BITCOIN_PID_FILENAME)
But this is not ideal because nothing checks that setting names are spelled correctly, or that default values (BITCOIN_PID_FILENAME
) are used consistently in the help string and retrieval sites, or that settings are retrieved with consistent types (bool, int, string, path, or list). This PR addresses these issues by adding setting declarations which allow settings to be registered and retrieved like:
0// Declare setting
1using PidSetting = common::Setting<
2 "-pid=<file>", fs::path, {.legacy = true},
3 "Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)">
4 ::DefaultFn<[] { return BITCOIN_PID_FILENAME; }>;
5
6// Register setting
7PidSetting::Register(argsman);
8
9// Retrieve setting
10PidSetting::Get(args)
Suggestions for review
All the real changes in this PR are in the last scripted-diff commit:
If you take a few minutes to look at the changes applied in this scripted-diff commit, you’ll understand everything this PR is doing. A good place to start looking around in this commit is the generated src/init_settings.h
file which declares the settings that get registered in src/init.cpp. Then you can look at the surrounding diffs and see they are just replacing AddArg
and GetArg
calls.
The other notable commit is second commit, which implements the Setting
class:
The other commits do minor things like moving code or updating linters. The python script that implements the scripted diff is a temporary artifact that gets deleted. The python script is complicated because it does things like parsing c++ code to extract help strings, and figuring out the right types to declare settings with so code compiles. But the entire scope of the script is to (1) generate Setting type definitions, (2) add #includes, and (3) replace AddArg()
calls with Register()
calls and GetArg()
calls with Get()
calls. So there is not much the script can actually do wrong without triggering build and test failures.
Extensions
This PR only adds the ability to declare individual settings with built-in types. It doesn’t provide any new runtime behavior, but a branch in issue #22978 extends the Setting
class implemented here to support runtime setting validation, additional types like std::variant
, additional conversion options, custom types, custom validation, and groups of settings declared as options structs.
This change is mostly orthogonal to #16545. #16545 only provides runtime type checking while this PR only provides compile-time checking with no new runtime behavior. But this change does allow a nicer way of declaring types in #16545, using c++ types like int
instead of flags like ALLOW_INT
, orstd::vector<std::string>
instead of ALLOW_STRING | ALLOW_LIST
.