Originally posted by @kiminuo in #22766 (review)
I wonder whether enum class Flags : uint32_t may be used here as Jon did in https://github.com/bitcoin/bitcoin/pull/21506/ (guideline). Would it make sense to you in this or a follow-up PR?
ryanofsky #22766 (review)
This could be fine, but I’m not sure what the specific advantage of doing it would be in this case (not sure what ways it may be safer). I do think it’s basically tangential in any case, and could be better addressed in a standalone PR.
kiminuo #22766 (review)
I think the rationale in the guideline makes sense and that’s why I suggested it. There was no specific advatange I had in mind. Anyway, it may (not) be a good follow up PR.
ajtowns #22766 (review)
See NetPermissionsFlags in net_permissions.h for comparison; it gives you lots of namespacing (so you’d be writing ArgsManager::Flags::ALLOW_BOOL everywhere) and means you can’t just treat them as ints (so bitwise or’ing things together means you have to declare your own operator)
jonatack #22766 (review)
Yes, two sides of a coin, I suppose.
Namespacing can allow shortening the enumerator names, since they no longer need to be unique in global namespace (and can therefore no longer need to be shouty “there can only be one”), e.g. ArgsManager::ALLOW_STRING could be ArgsManager::Flags::string. Whether that naming is better may be personal taste, but getting them out of global space seems good.
Using an enum class permits defining allowed operations: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-oper and https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-class (“minimize surprises: traditional enums convert to int too readily”). Changing NetPermissionFlags to an enum class found a bug related to this reason.