kfogel@collab.net wrote:
> Philip Martin <philip@codematters.co.uk> writes:
>> Daniel L. Rall wrote:
>>>On Thu, 29 Sep 2005, Philip Martin wrote:
>>>
>>>>SVN_NEON_0_25 is an AC_SUBST value in svn_private_config.h and it's
>>>>either a #define to 1 or it's completely absent. Looking through the
>>>>code we test for it using #if, should we be using #ifdef instead? I
>>>>know it makes little difference at the moment, but we do use #ifdef
>>>>for things like SVN_HAVE_OLD_EXPAT and SVN_LIBSVN_FS_LINKS_FS_BASE.
>>>
>>>#ifdef
>>
>>I see that this was discussed before and that Karl expressed a
>>preference for #if
>>
>>http://svn.haxx.se/dev/archive-2005-06/0511.shtml
>
> Thanks for digging that up, Philip.
Yes, thanks. I need to reiterate what I said in that thread, though I now
think there was perhaps a little disconnect between what Karl thought I wanted
and what I thought he wanted such that we are not actually arguing against each
other.
Karl argues for using "#if [!]" rather than "#if[n]def".
I argue for using either "#define FOO [0|1]" with "#if [!]FOO", or
"#define/#undef" with "#if[n]def".
We can both have our way if we always define the symbol as "0" or "1", and
always use "#if [!]" to test it.
> Julian Foad <julianfoad_at_btopenworld.com> writes:
>> >>By the way, the fact that the r15051 conditional compilation guards
>> >>use "#if" not "#ifdef" should be okay: any macro name remaining
>> >>after expansion is replaced with 0L by the preprocessor, so if
>> >>SVN_NEON_0_25_0 isn't defined at all, then all those conditionals
>> >>should simply evaluate to false.
>> > See also K&R p. 232.
>>
>> That's all true, but I consider it bad practice to make use of this
>> fallback behaviour in circumstances where the definition of the
>> variable is under our control and is therefore known to be of either
>> the defined/undefined kind or the 1/0 kind at our option. Setting the
>> variable according to one scheme and testing it according to the other
>> scheme causes unnecessary confusion for humans, and makes the
>> associated compiler warnings (where available) useless for detecting
>> real bugs because they have to be turned off or ignored.
>>
>> Well, that's my three-penn'orth.
I stand by what I said. To do otherwise does not help anyone.
Karl wrote in reply:
> Really, the inconsistency starts in svn_private_config.h, where the
> toggling is between this
>
> /* Is FOO support enabled? */
> /* #undef SVN_HAVE_ZLIB */
>
> and this
>
> /* Is FOO support enabled? */
> #define FOO 1
Yes. Those examples are unclear, but basically what we both mean is that FOO
should be set either by "#define FOO"/"#undef FOO" or by "#define FOO
0"/"#define FOO 1", and not a mixture of the two styles.
> But as for the tests in the code, I actually prefer to use "#if",
> because that way it means what you want it to mean even if someone
> sticks "#define FOO 0" into a header somewhere.
Invalid argument. Someone only sticks "#define FOO 0" into a header if they
already know that we are treating the symbol as having a defined value
(true/false). If they know or suspect that we are treating it as
define/undefined type, they would stick "#undef FOO" instead.
> In other words, "#if FOO" Does The Right Thing both when FOO is not
> defined at all, and when it is defined to be 0.
Similarly, "#ifdef FOO" Does The Right Thing both when FOO is defined as "1"
and when it is defined as "YES". In order to make your idea into an argument,
you would have to say why it is especially convenient to be able to set FOO
both to undefined and to 0 for the same effect, more so than my "1"/"YES" idea.
> (I suspect this is
> why the preprocess has this behavior in the first place.)
In some sense, perhaps: I think there was no "#undef" in some pre-standard
implementations, and the standard wanted to support code already written for
them. Thus I think it's a backward-compatibility feature, not one that is
meant to be used in new code.
>
> Of course, then the tests for the opposite sense should probably look
> like this
>
> #if ! SVN_NEON_0_25_0
Definitely.
> instead of this
>
> #ifndef SVN_NEON_0_25_0
>
> the latter being what I actually committed. I admit to inconsistency
> there :-). But if we're going to change something, it should be that,
> not the "#if"s.
OK, but it should be that AND the definition of the symbol.
> -Karl
And recently, in the present thread, Karl wrote:
> I'm +1 on changing the opposite tests to "#if ! PREPROCESSOR_CONSTANT"
> instead of "#ifndef PREPROCESSOR_CONSTANT" wherever applicable, of course.
Good: that will at least make the usage self-consistent. Let's at the same
time make the definition consistent with the usage.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 7 14:15:23 2005