On Mon, Feb 26, 2024 at 04:56:28PM -0800, Kyle Lippincott wrote: > > We use inttypes.h by default because the C standard already talks > > about it, and fall back to stdint.h when the platform lacks it. But > > what I suspect is that nobody compiles with NO_INTTYPES_H and we > > would unknowingly (but not "unintentionally") start using the > > extended types that are only available in <inttypes.h> but not in > > <stdint.h> sometime in the future. It might already have happened, > > It has. We use PRIuMAX, which is from inttypes.h. Is it always, though? That's what C99 says, but if you have a system that does not have inttypes.h in the first place, but does have stdint.h, it seems possible that it provides conversion macros elsewhere (either via stdint.h, or possibly just as part of stdio.h). So it might be that things have been horribly broken on NO_INTTYPES_H systems for a while, and nobody is checking. But I don't think you can really say so without looking at such a system. And looking at config.mak.uname, it looks like Windows is such a system. Does it really have inttypes.h and it is getting included from somewhere else, making format conversion macros work? Or does it provide those macros elsewhere, and really needs stdint? It does look like compat/mingw.h includes it, but I think we wouldn't use that for msvc builds. > I think it's only > "accidentally" working if anyone uses NO_INTTYPES_H. I changed my > stance halfway through this investigation in my previous email, I > apologize for not going back and editing it to make it clear at the > beginning that I'd done so. My current stance is that > <git-compat-util.h> should be either (a) including only inttypes.h > (since it includes stdint.h), or (b) including both inttypes.h and > stdint.h (in either order), just to demonstrate that we can. It is good to clean up old conditionals if they are no longer applicable, as they are a burden to reason about later (as this discussion shows). But I am not sure about your "just to demonstrate we can". It is good to try it out, but it looks like there is a non-zero chance that MSVC on Windows might break. It is probably better to try building there or looping in folks who can, rather than just making a change and seeing if anybody screams. I think the "win+VS" test in the GitHub Actions CI job might cover this case. It is not run by default (because it was considered be mostly redundant with the mingw build), but it shouldn't be too hard to enable it for a one-off test. -Peff
