| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I'd like @NattyNarwhal's opinion on this. |
Sorry, something went wrong.
|
I think it'd still trigger on MSVC regardless; what'd probably be better if the build system checked for stdint's existence and decided to use the reimplementation that way. (My tendency would be an "if stdint.h include stdint.h else reimplementation", and have all of lg2 include git2/stdint.h which does that. OFC, you'd have to make sure the include path never includes include/git2 directly, or you're in include_next hell.) Which would help non-MS C89 only compilers. (i.e. anyone wanting to port to the othjer really weird/old platforms.) |
Sorry, something went wrong.
This is pretty close to the status quo, I think. Right now common.h includes (our) stdint.h iff you're _MSC_VER < 1800. In other words, if we need to define uint64_t so that we can use it in a prototype. The change here is to remove the error if somebody not on that platform includes this file. I am not super convicted about this error, I guess. People should not be including git2/anything.h directly, they should always just include git2.h. So this was really to prevent people wrongly including git2/stdint.h and now it will just be a noop for them. I sort of liked the #error, because it is really wrong to include git2/anything.h and this was a good notice for that. I guess. But probably nobody ever included git2/stdint.h (even if they were including things underneath git2/, they were probably never including this one - including git2/merge.h because you want to do a merge makes sense -- including this file doesn't.) So I guess it's not really a particularly useful #error to have around. I must admit that I'm a little curious about the whole swift thing. I sort of don't understand how swift packages are building us here, because why would they ever hit this error and why do we need to remove it for them? But in this particular instance it doesn't seem so problematic, so this remains more an idle curiosity than a true complaint. |
Sorry, something went wrong.
|
Anyway, to make explicit a point that I only alluded to - and one that I always forget about when I see this file and it takes me a while to page this knowledge back in: this file needs to exist so that we can talk about the stdint types publicly (eg, a lot of functions take a size_t and that doesn't exist on MSC_VER < 1800 unless we define it. So that's what we're doing here. |
Sorry, something went wrong.
|
I'm pretty sure size_t is defined as part of C89, so you can not worry about that. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR hides header content from non-Windows builds.
( Well, it is better to include this header via "relative" search path ).