At 03:25 PM 19/10/2005 -0400, David James wrote:
>On 10/19/05, Branko ??bej <brane@xbc.nu> wrote:
>> David James wrote:
>> >On 10/19/05, Marc Haesen <Marc.Haesen@telindus.be> wrote:
[snip]
>> >This is fantastic news, Marc. I have followed your suggested approach,
>> >using pool = pool, to avoid a warning in r16817. Thanks so much for
>> >your help and your patience!
>>
>> I'd really like to know what sort of Windows compile problem the "(void)
>> pool" thing caused. This is an idiom we've been using throughout the
>> code to silence warnings about unused variables. Changing this single
>> isntance is nonsense, especially since the "compile problem" report
>> wasn't exactly enlightening.
>
>It does seem strange that "(void) pool;" would not work on Windows,
>especially considering how often we use the same idiom elsewhere in
>the code:
>
>subversion/libsvn_subr/config.c: (void)(baton); /*
>Unused parameter. */
>subversion/libsvn_subr/config.c: (void)(section); /*
>Unused parameter. */
>subversion/libsvn_wc/adm_files.c: (void)pool; /* Silence compiler
>warnings about unused parameter. */
>subversion/libsvn_wc/adm_files.c: (void)pool; /* Silence compiler
>warnings about unused parameter. */
It seems odd to me to use this directly in the code. It is essentially
relying on an undocumented feature of the compiler to work around an
unspecified (as far as I know) portion of the language (specifically
whether a warning should be emitted in this case, and what should be done
to prevent the warning). I wouldn't be at all surprised if there are
compilers out there that would take one look at "(void)pool" and say "...
that doesn't do anything!" In any event, the presence of this hack
certainly doesn't do anything positive for the appearance & readability of
the code.
If I were writing Subversion from scratch, I would use a macro for this. It
permits the behaviour to be turned on and off or changed altogether
depending on the compiler, which is as it should be since the warning is
compiler-specific as well.
Supposing "(void)pool" really was the source of the problem in Windows (I
don't see how it could be, but...), perhaps something like this would make
sense somewhere in a core header:
#if NEED_TO_SUPPRESS_UNUSED_PARAMETER_WARNING
# if defined(WIN32) || defined(WIN64)
# define UNUSED_PARAMETER(x) ((x) = (x))
# else
# define UNUSED_PARAMETER(x) ((void)(x))
# endif /* WIN32 || WIN64 */
#else
# define UNUSED_PARAMETER(x)
#endif /* NEED_TO_SUPPRESS_UNUSED_PARAMETER_WARNING */
Then, the code that had unused parameters would look like:
void fuu(void *baton, int bar, double qux, ...)
{
UNUSED_PARAMETER(baton);
...
}
That's just my take on it :-) It makes the behaviour centralized,
standardized and easy to change, and its usage self-descriptive, obviating
the need for an explanatory comment of the type shown above by David James.
Jonathan Gilbert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 19 21:44:00 2005