On Mar 9, 2010, at 4:46 PM, Blair Zajac wrote:
> On 03/09/2010 02:41 PM, hwri...@apache.org wrote:
>> Author: hwright
>> Date: Tue Mar 9 22:41:16 2010
>> New Revision: 921181
>>
>> URL: http://svn.apache.org/viewvc?rev=921181&view=rev
>> Log:
>> JavaHL: Fix a few header files to avoid a redundant declaration of SVN::Pool.
>> Instead of declaring the class (when it might also be declared previously by
>> some other header file), just include the header if needed. The header
>> already
>> has the required #ifdef protection, and it doesn't cost much to parse it
>> again anyway.
>
> Standard practice is to only include header files one needs for the
> definition. Plus, it's not just that, but the cost of recompiling everything
> that then #include's that header file if you touch Pooo.h
I try to stay as far away from Pooo.h has possible. :)
In all seriousness, when working in the Java bindings, I spend a fair amount of
time tracking down warnings and making sure they aren't errors. g++ can be
verbose as it is; I'm just trying to make my life a little easier by
eliminating a number of superfluous warnings. Since I seem to be the only
person hacking JavaHL these days, I think this little bit of selfish indulgence
is justified.
> I suggest just reverse merging this change, as while it's not the cost that
> matters (which is low as you state), but just being correct about it.
This point I understand. The problem happens when the class isn't declared in
the proper order in the header sequence, and we end up with something that
expands to:
SVN::Pool {
...
};
...
SVN::Pool;
The last line creates a warning, but only occasionally, depending on where the
declaration falls in relation to various other includes (which are often in
other files). The available options are:
1) Status quo
2) r921181
3) Wrap all SVN::Pool declarations with #ifdef's to avoid duplicate declaration
warnings
4) Audit the entire header file corpus to ensure all declarations are properly
ordered.
I chose (2), which gives the benefits desired with the least amount of work.
The problem really goes much farther than SVN::Pool: we include a number of our
header files in other header files. To properly solve the problem requires
(4); reverting r921181 doesn't really fix anything, it's just shuffling deck
chairs.
If anybody wants to tackle (4), please feel free.
-Hyrum
