Differential D159045
[clang-tidy] Improved documentation for readability-function-size Authored by felix642 on Aug 28 2023, 5:36 PM.
Details The documentation would say that that default value for most parameters is -1. If a user tried to use -1 to disable this check he would receive an error saying that
Diff Detail
Event TimelineComment Actions Change is ok. Interesting thing is that we got more of such issues in many checks. I'm thinking, maybe support for optional fields is needed, fields that can accept integer or false/no values. Comment Actions We can already disable those options if we don't define them in the config. Adding the possibility to provide optional values seems redundant to me. Do you see any reason why we would absolutely need to add this option to the config if we want to disable it? Comment Actions a) If you use config file inheritance, when one config file define it, and other that inherit it (or command line) want to disable such option. Comment Actions In that case, I agree with you, it would be helpful to add this feature. I think supporting an empty value rather than a boolean is preferable. We should maybe do that in another Differential though. I can open an issue on github and I'll open another diff when I'm ready. What do you think? Comment Actions Yes this should be done as a separate change. template <typename T>
std::enable_if_t<std::is_integral_v<T>, std::optional<T>> get(StringRef LocalName, std::optional<T> Default) const {
if (std::optional<StringRef> Value = get(LocalName)) {
if (value == "none" || value == "null" || value == "false" || value == "") // and -1 for unsigned types for backward combability
return std::nullopt;
T Result{};
if (!StringRef(*Value).getAsInteger(10, Result))
return Result;
diagnoseBadIntegerOption(NamePrefix + LocalName, *Value);
}
return Default;
}And storing would need to be updated in similar way. |