See individual commit messages for description.
Many of these warnings should be quickly fixed or worked around rather than disabling them, but I'd rather not mix code changes in with these, and do those as a next step if this is desired.
The whitespace changes in the .h make the diff hard to read, but the change is simple: if we're building internally, use the autoconf checks. Everything else is moved to the else block.
If building internally, autoconf's checks are used to determine exactly what is
supported by the compiler. If external, fall-back to the previous logic.
This allows for warnings to be added/removed even if they're only supported by
some compilers. Clang's -Weverything is a good example.
After turning on Clang's -Weverything, lots of new warnings are reported. They
have been disabled with the necessary flags, but it may be worth cleaning up
some of them and re-enabling those warnings.
I realize that this is a bit overkill for now, btw. I wanted to add it at this point because as more and more function attributes/built-ins/intrinsics are added, it gets harder to maintain what works and where. By checking on actual available functionality from the start, those headaches can be avoided.
NO NO NO.
You CANNOT USE AUTOCONF TO CONTROL COMPILER FEATURES IN A PUBLIC HEADER.
The function annotations are API features provided to callers, which will not be including the header from our own autoconfigure context. The method I put in is highly reliable and is tested on some 30 platforms with dozens of compilers and has never had an issue. Having autoconf handle this will not work.
@gmaxwell hmm? No problem if you don't wish to use this approach, but please re-read the changes. Callers will see the same as before.
The autoconf'd macros apply only to our build, callers won't have SECP_INTERNAL and HAVE_CONFIG_H defined.
Sorry, then github is mangling the diff, looked to me like it's just removing the normal detection and replacing it with autoconf.
Whats the gain there? It adds a lot of complexity. Really I think there should be no hint of autoconf in the public header, or at least there should be a good reason for it.
WRT the warnings, I don't have as much of an opinion about testing them one at a time... I intentionally didn't to avoid slowing down ./configure. The most important thing is that they're enabled for developers/maintainers of the library. (Usually heavy warnings for regular users are slightly problematic, e.g. when clang adds a (potenitally misguided) proflatic precidence/parrens warnings, and you end up with users "worried" that something is wrong with the library... but they're tolerable just because you will get some important warnings from users on weird platforms.)
WRT the existing clang warnings, looks like all of them are related to GNU99 or even just C99 features (e.g. VLA) that we're intentionally using (not that I want to be... I'd strongly prefer the library be C89). Not sure why it's warning about features that have been specifically enabled though. Perhaps we should be filing clang bugs on that.
uh actually, after looking at the Weverything output, I don't think we should be using this. It's emitting warnings on all kinds of totally normal C behavior. We can go -Wno-foo it, but what happens when the next version of clang adds more of this kind, and users are flooded with scarry warnings?
Maybe in travis-only rather than the distributed system?
For the header, the gain is being able to use more obscure attributes/builtins from newer compilers when possible. If you don't anticipate that, and you've had no problems with your macros, then you're right to call it needless.
Wrt clang's -Weverything, sure. It'd probably make sense to build that way, see what it found, then manually turn things on instead. Most of it is useless, but some of the cast/overflow/convert warnings can be really helpful when dealing with primitive data.
ok, closing since the bulk of this isn't worth doing. I'd like you to consider something like the warnings change, though, mainly so that clang-specific ones could be added.