Hi Eli, Sorry for the very late response, but the last months were a bit busy for me (graduating, migrating).
2011/12/15 Eli Friedman <[email protected]>: > I'm pretty sure we call CheckCompleteVariableDeclaration for both > those declarations; I could be mistaken, though. I just checked and it seems CheckCompleteVariableDeclaration is not called for those declarations. Maybe it's only called for variable declarations with an initializer? >> I'm not a very good compiler hacker so I could use some help here. I >> suspect you mean something like this? >> >> if (var->isThisDeclarationADefinition() && >> var->getLinkage() == ExternalLinkage && >> var->getPreviousDeclaration() == NULL) >> Diag(var->getLocation(), >> diag::warn_missing_variable_declarations) << var; > > Yes. It seems getPreviousDeclaration() has been removed, so I've fallen back to simply checking against `Previous.empty()'. If anyone knows of a more elegant solution, please let me know. The latest version of the patch can be found here: http://80386.nl/pub/wmissing-variable-declarations.txt As this thread went a bit stale over the last couple of months, let me rehash what this patch is about. The -Wmissing-variable-declarations flag adds a warning for the following: int i; // Global variable declaration that has no prior extern declaration and is not static: warn static int j; // Don't warn extern int k; int k; // Don't warn One can say it's similar to -Wmissing-prototypes, but then for variables. In my opinion it is good to have such a warning flag for the following reasons: - It makes it easier to find dead code, namely unused global variables. If you get one of these warnings and decide to mark the variable static, you will immediately get a compiler warning about an unused global variable. - There are cases in which marking as many things static has a positive impact on the number of optimisations that can be performed. For example, consider the following piece of code: int nresults; void findresults(void) { nresults = 0; somefunction(); while (nresults == 0) { // find results } } Because `nresults' is not static, the compiler cannot rely on `somefunction()' to leave `nresults' alone. By marking it static (and under the assumption that a pointer to `nresults' never leaves the compilation unit), the compiler is free to optimize the while-loop to a do-while-loop. - People often forget to use const-keywords properly: const char *myblacklist[] = { "apples", "oranges", NULL }; When this array can be marked static, Clang is smart enough to place the array in a read-only segment when it detects that the contents of the array are not modified This may potentially be a good thing from a security point of view. - It may in some cases find real bugs in your code. C (maybe just ELF?) has this `awesome' feature that it merges global variables if they have the same size (and are not initialized multiple times). If one C file contains `long x;' and the other one contains `double x;', all sorts of funny things might happen. This warning flag could spot issues like this. I used this patch set last year on the FreeBSD source tree and I observed that simply sprinkling some more static-keywords throughout the code of code had some positive impact. Some highlights: - stty(1): though the binary is only 22 KB in size, 3.5 KB of data in the binary section became read-only. - The size of the tools in /bin decreased by approximately 1%. Thanks for your patience, -- Ed Schouten <[email protected]> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
