On 2022-12-01 01:27, John Hawkinson wrote:
I don't think our test coverage is strong enough to ensure that these kinds of changes do not introduce problems.
Yes, that goes without saying, for almost any project, as testing doesn't 100% guarantee correctness. The idea isn't to go out and use every C99 feature just because we can. It's to improve code quality by not insisting on portability to C89. And these improvements need not be limited to new code; they can also apply to existing code. This is not a new thing. We did something similar in 2012 when we started assuming C89 or later. For example, this patch: https://github.com/eggert/tz/commit/400ecf36bb0b73f6390f9641e6cb8bbfb91a5cfd changed tzcode to use function prototypes instead of K&R style function declarations. If the policy had been to not refactor unless we could guarantee it wouldn't introduce problems, then that patch would not have been applied in 2012, since the old code ran just fine on C99 and C11 platforms. However, the patch was a win because C89 function prototypes have important technical advantages over K&R style. Although there was some immediate pain due to this conversion, in the not-so-long run it was a win. (It'll be an even bigger win once C23 becomes popular, as C23 outlaws K&R function declarations.) We'll have similar opportunities once we start assuming C99. We can decide which C99 features to use on a case-by-case basis. We shouldn't use every possible C99 feature we can, only the ones where the long-term benefit is arguably worth the short-term cost. For example, tzcode should not use C99's variable-length arrays because they're not that portable in practice. (They were so controversial that they even became optional in later C standards, so we couldn't use them anyway.) In contrast, the C99 feature of declarations after statements can help code readability significantly, and if someone (probably me) wants to take the time to convert existing code to use this feature, that sounds like a win. And there's quite a bit of backward compatibility code (e.g., our own implementation of snprintf) that could be removed, as in practice nobody uses or tests it any more and it's simply a maintenance burden. I wouldn't object to removing that code, quite the contrary. I understand that doing all this will place a burden on reviewers, and will make spelunking harder. But that's OK if it lessens the overall maintenance burden in the long run.