Remove public 'inttypes.h' header#4991
Conversation
|
Tested on Visual Studio 2012 (aka |
pks-t
left a comment
pks-t
left a comment
There was a problem hiding this comment.
Just a request for claring up my own lack of knowledge. Definitely looks good to me, though. Always happy to remove unneeded code :)
| #if defined(_MSC_VER) || defined(__MINGW32__) | ||
|
|
||
| /* Visual Studio 2012 and prior lack PRId64 entirely */ | ||
| # if (_MSC_VER < 1800) |
There was a problem hiding this comment.
Wouldn't it be easier to just check if PRId64 is defined instead of relying on the _MSC_VER? No idea though if MSVC is using e.g. a typedef, but I guess you'd know :)
There was a problem hiding this comment.
How would you implement PRId64 using a typedef (in C)? AFAIK the only way to implement it is by using preprocessor-macros.
|
On Wed, Feb 20, 2019 at 02:27:34AM -0800, Julian Ganz wrote:
neithernut commented on this pull request.
> @@ -48,6 +48,11 @@
/* Define the printf format specifer to use for size_t output */
#if defined(_MSC_VER) || defined(__MINGW32__)
+/* Visual Studio 2012 and prior lack PRId64 entirely */
+# if (_MSC_VER < 1800)
How would you implement `PRId64` using a _typedef_ (in C)?
AFAIK the only way to implement it is by using
preprocessor-macros.
Oops. Due to reading through the "inttypes.h" header I've somehow
thought of `int64_t` etc.
So given that this is a clear "No, this is not implemented via a
typedef": wouldn't it make sense to just use an `#ifndef PRId64`
instead of intimately checking the MSVC version?
|
ethomson
force-pushed
the
ethomson/inttypes
branch
from
0af23aa to
56bbb7e
Compare
Agreed. Fixed! |
Remove an `inttypes.h` header that is too large in scope, and far too public. For Visual Studio 2012 and earlier (ie, `_MSC_VER < 1800`), we do need to include `stdint.h` in our public headers, for types like `uint32_t`. Internally, we also need to define `PRId64` as a printf formatting string when it is not available.
ethomson
force-pushed
the
ethomson/inttypes
branch
from
56bbb7e to
247e6d9
Compare
|
Cool, thanks for this cleanup! |
3 participants
Remove an
inttypes.hheader that is too large in scope, and far toopublic.
For Visual Studio 2012 and earlier (ie,
_MSC_VER < 1800), we do needto include
stdint.hin our public headers, for types likeuint32_t.Internally, we also need to define
PRId64as a printf formattingstring.