Re: [PATCHv4 6/6] Remove forward declaration of an enum
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Aug 15, 2018, 20:40 UTC
- Message-ID
- <20180815204030.GC181377@aiede.svl.corp.google.com>
- In-Reply-To
- <20180815175410.5726-7-newren@gmail.com>
Elijah Newren wrote:
Show 8 quoted lines
> According to http://c-faq.com/null/machexamp.html, sizeof(char*) != > sizeof(int*) on some platforms. Since an enum could be a char or int > (or long or...), knowing the size of the enum thus is important to > knowing the size of a pointer to an enum, so we cannot just forward > declare an enum the way we can a struct. (Also, modern C++ compilers > apparently define forward declarations of an enum to either be useless > because the enum was defined, or require an explicit size specifier, or > be a compilation error.)
Beyond the effect on some obscure platforms, this also makes it possible to build with gcc -pedantic (which can be useful for finding some other problems). Thanks for fixing it.
[...]
Show 12 quoted lines
> --- a/packfile.h > +++ b/packfile.h > @@ -1,12 +1,12 @@ > #ifndef PACKFILE_H > #define PACKFILE_H > > +#include "cache.h" > #include "oidset.h" > > /* in object-store.h */ > struct packed_git; > struct object_info;
Not about this patch: comments like the above are likely to go stale, since nothing verifies they continue to be true. So we should remove them. #leftoverbits
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>