I don't understand this change. What is the added value here ? It just makes the api counter intuitive imo

tldr; the current typing system is basically just descriptive but not enforced. The only check that is being done is that INT args are not negated.
It would be good to be able to switch to stronger typing in args, but doing so would likely break many user settings, so it will have to be done carefully on existing args. This change reflects the current state of type checking: most args can be of any type.

Relevant context from the PR description:

@amitiuttarwar :
Do I understand the idea & context correctly? - enforcing type validation is the end goal & this is an intermediary step to have the code reflect the level of validation that's actually currently occurring (aka, not much)?

@ryanofsky:
Yep, that's all correct but just to be clear this PR has two goals:

• Get rid of misleading ArgsManager flags
• Simplify future PRs implementing more validation by cleaning up existing validation

The PR should make sense by itself just for the first goal even if you don't care about the second goal.

Maybe another helpful note about the PR is that only the first commit adds any new code. The other two commits are scripted diff cleanups that don't add any code.

@hebasto:
Wondering what are benefits of the DISALLOW_NEGATION flag in comparison to the ALLOW_NEGATED one (not an issue at all, just trying to understand motivation)?

@ryanofsky:
There are two types of options:

• Typed options (int, string, bool, list options with implementation under discussion in refactor: Implement missing error checking for ArgsManager flags #16545)
• Untyped options (ALLOW_ANY options)

For untyped options, I think DISALLOW_NEGATION is better than ALLOW_NEGATED because negation has always been allowed for these options. For the 200 or so existing untyped options that allow negation, this PR is leaving them untouched, not changing their behavior or the way they are declared. There are 5 untyped options that don't allow negation. This PR is changing the declaration of these 5 options to be less misleading and actually match their implemented behavior.

For typed options, I think DISALLOW_NEGATION should rarely or never be used. Almost all existing options accept negation. Negation obviously makes sense for bool options. Existing usage shows it also makes sense for most int and string options. For some int and string options, it doesn't make sense, but in these cases generic negation handling and the generic negation message "Negating of -xxx is meaningless and therefore forbidden" is worse than just writing validation logic that has to be written anyway and giving specific errors like "Setting -xxx value false is out of range, the acceptable range is YYY-ZZZ." "Setting -xxx value false is not recognized, acceptable values are A, B, and C."

If I'm wrong about this and the DISALLOW_NEGATION flag ever starts providing more useful messages, or starts being used many more places, it can be enabled by default in the future. But don't think these things will happen and I think even if they did happen later it would be premature to disallow negation by default and update 200 existing option declarations right now.

ryanofsky:
Thanks for review! Just wanted to quickly say you do not need to care anything about #16545 for this PR to make sense. This PR is disabling flags ALLOW_INT/STRING/BOOL which are broken, by commenting them out. #16545 is reenabling these flags by uncommenting them again and implementing them.

Even if #16545 is rejected, these flags are misleading now and should be disabled. The only effect these flags have now is to disallow negation, so this PR is replacing them with a clearer DISALLOW_NEGATION flag. I expect this flag to be rarely used in the future, and probably to be removed later. Defining it here is just useful to make current code clear and avoid changing behavior in any way with this PR.

The more immediate reason I'm backporting this is that the new flag is used for the new -loglevel= arg added in https://github.com/bitcoin/bitcoin/pull/25614

But I can also just skip it and just use ArgsManager::ALLOW_ANY instead