Learning C with Older books ?.

[Daniel Vallstrom]

[snip discussion about translating ordinary language into
a formal one.]

I can only agree with James Dennett's reply to your post and
I'll just reply to some of your comments. Over all IMO you
take a too positivistic approach. An essential --- and good! ---
characteristic of ordinary language is that it can *not* be
formalized. Sure, there might be a better formulation of what
-Wall mean, but it's no big deal.

You don't mean it doesn't make sense; you mean you disagree with what
it says. The meaning that "-Wall warns on all constructs that any user
found questionable" is perfectly sensible even if it happens to be

false.

I do mean "it doesn't make sense" (with the meaning that phrase usually
has in ordinary English). Let's agree that the sentence can mean one of
two things:

(1) warn about every character in the file;

(2) warn about this specific set of constructions.

Since (1) "doesn't make sense" (in the ordinary sense of the phrase),
(2) must be what the gcc devs mean.

Incidentally, you can tell a lot about the quality of someone's
arguments by their attitude toward using words like "luddite" as
part of their arguments.

Hm, reading that I get the feeling that you think that I in some way
implied a connection between "luddite" and you. Nothing could be
more wrong. Because surely you can't suggest that the word 'luddite'
should be banned? I used it in the following sense to get to (1) above:
"for every character there exists a luddite that wants to warn about
it" and "there exists a character that only a luddite wants to warn
about" Maybe there is a better word but I couldn't find one anyway.
And it doesn't stretch word meaning too far. I just tried to be
specific when I used the word 'luddite'. (Or maybe it was my
"today's word" that I had to use in a sentence

Again, if you think that I implied something disparaging between
the lines you are mistaken. It would also be somewhat ironic since
it is you that argues for a "logical" interpretation of English;p

It's also
possible to interpret it as "-Wall includes all the warnings for
constructs that GNU developers find questionable", and I wouldn't be
surprised if that interpretation were closer to the truth than most of
the others that have been mentioned.

That *is* the interpretation that all of us have been suggesting.



(Here is a link to the manual on gcc's warning options under
discussion

http://gcc.gnu.org/onlinedocs/gcc-3.4.3/gcc/Warning-Options.html#Warning-Options

OK, what are the problematic warnings triggered by -Wall?

I'm happy to go through the list of -W<something> options covered by
-Wall and see which ones I've found helpful and which not, if you will
also do that for the set of constructs covered by the -W option.

[stuff about -Wparentheses snipped]


-Wextra (-W) warns about the idiom

p0 == p1 == ... == pn

even when pi is 0 or 1, or of type bool. This is unfortunate.

Do you mean this to be the only warning covered under -W (aka -Wextra)
that you find objectionable?

I also listed -Wno-sign-compare which turns off -Wsign-compare
included in -Wextra. But other than that, the answer would be
"yes", with a quick reading.

But the fact that I --- like many others? --- also use -Wextra,
amended by -Wno-sign-compare, does not mean that I disagree
with anything stated on gcc's warning man-page about -Wall and
-Wextra.

The only thing I have against -Wall is the braindead &&-|| warning
and you can work around that problem without affecting the code
you write. It would be interesting to hear what you have against
-Wall. If it's the combination -Wall and -Werror that is the
problem then maybe you should skip -Werror instead of the (IMO
and in many others) useful -Wall.


Daniel Vallstrom

 

[Dan Pop]

Doing a Google Groups search with "-W" and "unsigned", and limiting
the time to Jan 1, 2003, to the present (and in comp.lang.c and with
(e-mail address removed) as author) yielded 11 hits. None of these contained
any substantive discussion of different -W options.

There is only one "-W" option as far as I know. And my main objection
to it is that I get the following kind of warnings:

fangorn:~/tmp 127> cat test.c
int main()
{
int i;

for (i = 0; i < sizeof(int); i++) ;
return 0;
}
fangorn:~/tmp 128> gcc -W test.c
test.c: In function `main':
test.c:5: warning: comparison between signed and unsigned

that cannot be shut up without using a programming style I do not like
(either use of unnecessary casts or proliferation of unsigned variables).

Of course, I could disable this specific warning with -Wno-sign-compare
but I can't find enough value in what's left to justify bothering with
baroque sequences of -W... options.

OTOH, I couldn't find any objectionable warning generated by -Wall when
dealing with correct code. In all cases, shutting such warnings up
results in code whose correctness is clearer than before. -Wall also
helped me get rid of the "implicit int in function definitions" habit
inherited from the K&R C days.

So, what's wrong with -Wall, from your point of view?

Dan

 

[Charlie Gordon]

There is only one "-W" option as far as I know. And my main objection
to it is that I get the following kind of warnings:

fangorn:~/tmp 127> cat test.c
int main()

int i;

for (i = 0; i < sizeof(int); i++) ;
return 0;
}
fangorn:~/tmp 128> gcc -W test.c
test.c: In function `main':
test.c:5: warning: comparison between signed and unsigned

that cannot be shut up without using a programming style I do not like
(either use of unnecessary casts or proliferation of unsigned variables).

Of course, I could disable this specific warning with -Wno-sign-compare
but I can't find enough value in what's left to justify bothering with
baroque sequences of -W... options.

A perfectly valid warning !
Think of this :

#include <stdio.h>
int main(void)


int i;

for (i = -1; i < sizeof(int); i++) ;
printf("%d\n", i);
return 0;
}

This program outputs -1 contrary to popular belief !

The solution to this problem is this :

#include <stdio.h>
typedef signed whatever ssize_t;
#define ssizeof(t) ((ssize_t)sizeof(t))
int main(void)


int i;

for (i = -1; i < ssizeof(int); i++) ;
printf("%d\n", i);
return 0;
}

No warnings, correct output.
It is really a shame that a cast to the signed version of size_t cannot be done
portably, and that ssize_t is not part of C99 (but it is part of POSIX).

 

[Arthur J. O'Dwyer]

{

Incidentally, Charlie, your newsreader seems to be treating the '{'
character as a cue to insert two blank lines and delete the '{'. You
might want to see what might be causing that; it's a very odd behavior.
(I'm basing that assumption on the way you deleted the '{' in both Dan's
code and your own.)

A perfectly valid warning !
Think of this :

#include <stdio.h>
int main(void) {
int i;
for (i = -1; i < sizeof(int); i++) ;
printf("%d\n", i);
return 0;
}

This program outputs -1 contrary to popular belief !

Not a /terribly/ popular belief, of course. Besides, nobody's
objecting to that particular warning; obviously in that case the user
is doing something stupid and ought to be warned. But in Dan's example,
the user quite clearly knows what he's doing; not only is '< sizeof foo'
a well-established C idiom, but it can be proven at compile time that
Dan's code does exactly what it would do with or without the extra cast.
So it is entirely /un/reasonable for GCC to emit bogus warnings about it!

The solution to this problem is this :

#include <stdio.h>
typedef signed whatever ssize_t;

I don't think 'whatever' is in my copy of the Standard. If
this version isn't portable without manual tweaking by a C guru for
the platform in question, I don't think Dan's going to like it.

[...]

It is really a shame that a cast to the signed version of size_t cannot
be done portably, and that ssize_t is not part of C99 (but it is part of
POSIX).

Yes. Personally, I use '-O2 -W -Wall' and just ignore the bogus
warnings. Other than that one, and an occasional "variable foo may be
used uninitialized," and GCC's annoying spiel discouraging the use of
'tmpnam', I can't recall any more bogus warnings enabled by '-W'. So
I find them not too difficult to ignore. YMMV.

-Arthur

 

[Lawrence Kirby]

On Tue, 14 Dec 2004 21:06:09 +0100, Charlie Gordon wrote:

....

....

A perfectly valid warning !

True, except when the code is clearly correct. Having to compromise your
progrmming style due to dumb compiler diagnostics is a problem.

Think of this :

#include <stdio.h>
int main(void)


int i;

for (i = -1; i < sizeof(int); i++) ;
printf("%d\n", i);
return 0;
}

This program outputs -1 contrary to popular belief !

Yes, that's an unfortunate in a lot of cases. However the question is
whether diagnosing potential problems like this is worth the falso
diagnostics for good code. Indeed a reasonable compiler should be able to
work out that i can't be negative in Dan's original loop.

The solution to this problem is this :

#include <stdio.h>
typedef signed whatever ssize_t;
#define ssizeof(t) ((ssize_t)sizeof(t))

You don't even need ssize_t

#define intsizeof(t) ((int)sizeof(t))

Since the code that uses this is testing against an int variable, the
programmer is requiring the size of the object/type to be representable as
an int. Otherwise the code could fail irrespective of whether the cast to
int here is well defined.

Alternatively casting to long or intmax_t as appropriate is as portable as
a created ssize_t, and probably no less efficient where constant
expressions are concerned (i.e. sizeof non-VLAs). Indeed it might work
in cases where ssize_t doesn't.

int main(void)


int i;

for (i = -1; i < ssizeof(int); i++) ; printf("%d\n", i); return
0;
}
}
No warnings, correct output.
It is really a shame that a cast to the signed version of size_t cannot
be done portably, and that ssize_t is not part of C99 (but it is part of
POSIX).

For all intents and purposes it can be done.

Lawrence

 

[James Dennett]

On Tue, 14 Dec 2004 21:06:09 +0100, Charlie Gordon wrote:

...




True, except when the code is clearly correct. Having to compromise your
progrmming style due to dumb compiler diagnostics is a problem.

But getting to *improve* your style because of viable compiler
diagnostics is a fine opportunity, no?

-- James

 

[pete]

But getting to *improve* your style because of viable compiler
diagnostics is a fine opportunity, no?

That's what I was thinking.
It's good to use operand types which match.

 

[Lawrence Kirby]

On Wed, 15 Dec 2004 21:37:13 -0800, James Dennett wrote:

....

But getting to *improve* your style because of viable compiler
diagnostics is a fine opportunity, no?

That depends on whether you consider cluttered to be a better style than
keeping it simple. There is a legitimate issue that the compiler is
warning about, but it doesn't occur in this case and it is easy to see
that.

Lawrence

 

[Charlie Gordon]

For all intents and purposes it can be done.

How can it be done ?
C99 doesn't tell us much about the size of size_t, so how can you convert a
size_t expression to its signed equivalent, assuming no overflow, and without
extending precision ?

(intmax_t)sizeof(e) in unacceptable because it potentially extends the
size and changes the semantics of the surrounding expression as in

sizeof(sizeof(e)) != sizeof((intmax_t)sizeof(e))

I advocate casting to (signed) as a lone adjective should do this, but it is
incompatible with current C99 semantics that specify (signed) to be quivalent to
(signed int).

What is your version ?

 

[Lawrence Kirby]

How can it be done ?

As I suggested previously in the article you responded to.

C99 doesn't tell us much about the size of size_t, so how can you convert a
size_t expression to its signed equivalent, assuming no overflow, and without
extending precision ?

It doesn't have to be the signed equivalent, a type at least as wide as
that will do just as well, if not better if it can represent all size_t
values. So the widest integer type supported by the implementation works.
I do accept that when a wider than necessary type is used in a complex
expression that type will tend to propagate through the expression, a form
of "poisoning".

If you felt it necessary you could in C99 use the preprocessor to find the
largest signed integer type whose positive range does not exceed that of
size_t.

Alternatively you can cast to a type such as int. That would be used where
other parts of the code (e.g. index variables) require that the size
be representable as an int so the code would break anyway if it isn't. You
are no better off using a "true" ssize_t in that respect because even that
isn't guaranteed to be able to represent all possible object sizes. C
allows the range of an unsigned integer type to be significantly larger
than the positive range of the corresponding signed type (not just approx.
double the size), so IMO ssize_t isn't a particularly useful concept.

(intmax_t)sizeof(e) in unacceptable because it potentially
extends the size and changes the semantics of the surrounding expression
as in

sizeof(sizeof(e)) != sizeof((intmax_t)sizeof(e))

When is this likely to be a problem in real code? If you want
sizeof(size_t) then write that, or ssizeof(size_t) or intsizeof(size_t) or
whatever.

I advocate casting to (signed) as a lone adjective should do this, but
it is incompatible with current C99 semantics that specify (signed) to
be quivalent to (signed int).

What is your version ?

Cast to the type of the signed variable or quantity the result will be
used with, probably. If your problem is selecting a suitable type for that
then you have to resort to the alternatives above.

There is no universally perfect solution to this, and that includes a
"true" ssize_t type. I'm happy to use size_t for my index variables and
deal with/avoid the fairly clear cut issues that arise from that. If you
have to count backwards (which is the minority of cases) you can use code
like

for (size_t i = (sizeof x)-1; i != (size_t)-1; i--)

or

size_t i = sizeof x;
while (i-- != 0)

or

for (size_t i = sizeof x; i != 0; ) {
i--;
...
}

Incidentally, this last approach also works naturally with pointers.

Lawrence

 

[Charlie Gordon]

Cast to the type of the signed variable or quantity the result will be
used with, probably. If your problem is selecting a suitable type for that
then you have to resort to the alternatives above.

that's not safe either :

int i;
....
i < (int)sizeof(x)

makes an explicit assumption about the type of i. It is not advisable.
For the very same reason 'p = malloc(sizeof(*p))' is safer than 'p =
malloc(sizeof(x))'.
If the type of i is later changed to long to accomodate for larger size_t, the
cast can go unnoticed and create very hard to find bugs.

There is no universally perfect solution to this, and that includes a
"true" ssize_t type. I'm happy to use size_t for my index variables and
deal with/avoid the fairly clear cut issues that arise from that. If you
have to count backwards (which is the minority of cases) you can use code
like

for (size_t i = (sizeof x)-1; i != (size_t)-1; i--)

Horrible, and probably relies on undefined behavior.

or

size_t i = sizeof x;
while (i-- != 0)

It is bad practice to call i something that is not an int, because a lot of
programmers will infer the wrong type in later expressions.

or

for (size_t i = sizeof x; i != 0; ) {
i--;
...
}

i > 0 works as well and is more natural.

Incidentally, this last approach also works naturally with pointers.

Agreed! Yet I would use > as well for pointers for the sake of readability.

 

[Michael Mair]

that's not safe either :

int i;
...
i < (int)sizeof(x)

makes an explicit assumption about the type of i. It is not advisable.
For the very same reason 'p = malloc(sizeof(*p))' is safer than 'p =
malloc(sizeof(x))'.
If the type of i is later changed to long to accomodate for larger size_t, the
cast can go unnoticed and create very hard to find bugs.

Well, then make this

my_ssize_t i;
.....
i < (my_ssize_t)sizeof(x)

with my_ssize_t defined in a header file.

Horrible, and probably relies on undefined behavior.

How so? sizeof x > 0, and the unsigned overflow of -1 is defined.
If you do not like the cast, you can leave it out.

It is bad practice to call i something that is not an int, because a lot of
programmers will infer the wrong type in later expressions.

Hmmm, we are not talking about Fortran.
It is traditional to use i, j, ... for loop iteration, yes.
IMO, if someone infers from "i is used to iterate through the
loop" that i were an int, then this is his or her personal problem.

I am doing most of my loops through arrays (or storage accessed via
pointer) using size_t loop variables which, as often as not, are called
i, j, ... These may be declared just for the loop or at the beginning
of some block.

i > 0 works as well and is more natural.

Indeed. However, as you were referring to traditions:
This may help reminding the "i is always int" programmer that
we are dealing with i of type size_t

Agreed! Yet I would use > as well for pointers for the sake of readability.

Umh, I am not sure whether you and Lawrence are referring to the
same sort of loop with pointers.


Cheers
Michael

 

[James Dennett]

On Wed, 15 Dec 2004 21:37:13 -0800, James Dennett wrote:

...




That depends on whether you consider cluttered to be a better style than
keeping it simple.

I don't. But I consider type mismatches to be a failure
to keep things simple. Simplicity, true simplicity, is
very valuable in programming, and surprisingly hard to
achieve.

There is a legitimate issue that the compiler is
warning about, but it doesn't occur in this case and it is easy to see
that.

Special cases are a form of complexity too. In the end,
though, this is a very small issue, reasonable people can
disagree on their assessments of what is better.

-- James

 

[Michael Wojcik]

i > 0 works as well and is more natural.

Where in nature does "i > 0" occur?

You may prefer it, but it is nothing more than a preference. Spare
us the bogus claims about what is "natural" C syntax.

Personally, I'd be just as happy with a plain "i" for the second "for"
expression. I know what it means; any competent C programmer knows
what it means; someone learning C either knows what it means or needs
to learn; no one else should be reading the code.

 

[Lawrence Kirby]

that's not safe either :

int i;
...
i < (int)sizeof(x)

makes an explicit assumption about the type of i. It is not advisable.
For the very same reason 'p = malloc(sizeof(*p))' is safer than 'p =
malloc(sizeof(x))'.
If the type of i is later changed to long to accomodate for larger size_t, the
cast can go unnoticed and create very hard to find bugs.

Whenever you cast you have to make sure the cast has the right type. Where
there is a way to avoid it then great but where there isn't you just have
to make do. There isn't an equivalent to sizeof(*p) available here. The
best you can do is a typedef used for the definition of i and the cast,
which may be reasonable in some contexts.

Horrible, and probably relies on undefined behavior.

It is a normal, well known and portable idiom for getting the largest
value an unsigned integer type can represent. In this case the loop
condition also works if the type happens to be signed. You could hide the
details behind a macro if you wanted but I don't really see the need.

It is bad practice to call i something that is not an int, because a lot
of programmers will infer the wrong type in later expressions.

I associate i more with "index" than "int" here. If you prefer to change
the variable name feel free.

i > 0 works as well and is more natural.

How so?

Agreed! Yet I would use > as well for pointers for the sake of
readability.

I'd stick to != because it shows clearly that i had to hit a value exactly
for the loop to terminate. Seeing > somebody might be led to believe that
the test would still work if you skipped some values. != more accurately
describes the situation which makes it more readable in this case.

Lawrence

 

[Tim Rentsch]

There is only one "-W" option as far as I know.

Sorry that this description was unclear. The word "constructs"
or "events" (which is gcc's choice, not mine) might have been
better. I meant the different code conditions that -W flags,
something like the following:

-W Print extra warning messages for these events:

o A function can return either with or without a
value.

o An expression-statement or the left-hand side of a
comma expression contains no side effects.

o An unsigned value is compared against zero with <
or <=.

o A comparison like x<=y<=z appears;

o Storage-class specifiers like "static" are not the
first things in a declaration.

o The return type of a function has a type qualifier
such as "const".

o A comparison between signed and unsigned values
could produce an incorrect result when the signed
value is converted to unsigned. (But don't warn
if -Wno-sign-compare is also specified.)

o An aggregate has a partly bracketed initializer.

o An aggregate has an initializer which does not
initialize all members.

Based on your comments below it's only the signed/unsigned comparison
that you object to. Is that right or are there others? For the
sake of discussion let's assume the above list gives the conditions
flagged by -W.

And my main objection
to it is that I get the following kind of warnings:

fangorn:~/tmp 127> cat test.c
int main()
{
int i;

for (i = 0; i < sizeof(int); i++) ;
return 0;
}
fangorn:~/tmp 128> gcc -W test.c
test.c: In function `main':
test.c:5: warning: comparison between signed and unsigned

that cannot be shut up without using a programming style I do not like
(either use of unnecessary casts or proliferation of unsigned variables).

I agree that these conditions can be annoying. I do consider them
useful however, both because of potentially incorrect results, and
because of the rules for how signs are dealt with in integral
promotions is one of the few areas where ANSI is at odds with many
(most?) pre-ANSI compilers; I think it helps to raise consciousness
about that. (The ANSI decision to use "value preserving" rather than
"signedness preserving" as the semantics for these promotions was not,
IMO, the best choice; but whether it's a good choice or not is
incidental to considering the warning useful.)

It's nice to see that the comments about the signed/unsigned warning
condition has generated some good discussion further along this
thread.

Of course, I could disable this specific warning with -Wno-sign-compare
but I can't find enough value in what's left to justify bothering with
baroque sequences of -W... options.

Your regular compilation environment must be different from mine. I
hardly ever either type in or see any of the various compiler flags
that are set for essentially all compilations I do. So even if the
benefit were small, in my environment it would be worth the investment
which is essentially zero.

OTOH, I couldn't find any objectionable warning generated by -Wall when
dealing with correct code. In all cases, shutting such warnings up
results in code whose correctness is clearer than before. -Wall also
helped me get rid of the "implicit int in function definitions" habit
inherited from the K&R C days.

Certainly I agree that many or most -Wall warnings are worth
getting rid of, with the particular case of implicitly 'int'
function being one of them.

So, what's wrong with -Wall, from your point of view?

Ok, the $64 question. Again for the sake of discussion let's take
the list below as the set of warnings covered by -Wall:

-Wchar-subscripts - using values of type 'char' to subscript
-Wcomment - when /* appears in a comment string (& more)
-Wformat - check printf et al format strings against args
-Wnonnull - NULL arg to "nonnull" attribute parameters
-Wimplicit - decl w/o type, or function call w/o decl
-Wmain - type of main() is suspicious
-Wmissing-braces - initializers not fully bracketed
-Wparentheses - eg, if(x=a+b). also, some unpaired if's
-Wsequence-point - possible violations of sequence point rules
-Wreturn-type - non-void function 'return;'; no function type
-Wswitch - switch( ENUM ) that doesn't cover all cases
-Wswitch-default - switch() w/o 'default:' case
-Wswitch-enum - switch( ENUM ) cases != ENUM values
-Wtrigraphs - if any trigraphs are encountered
-Wunused - unused variable, value, label, static function
-Wunused-parameter - unused parameter
-Wuninitialized - variable might be used without having been set
-Wunknown-pragmas - unrecognized #pragma
-Wstrict-aliasing - code that may break with -fstrict-aliasing

I realize this set probably doesn't exactly match the gcc that anyone
is currently using, but I think it will help the discussion if we
proceed under the assumption that these conditions make up the current
set of -Wall conditions.

I would break these down into four categories - the always useful, the
usually useful, the probably useful, and the objectionable.

The 'always useful' are those that simply never get turned off (that
is, in my environment). These conditions are ones I consider
synonymous with errors. (Normally I expect -Werror will be set [1]
whether the compilation is a "development" compilations or a
"production" compilation.) The always useful ones are:

-Wchar-subscripts - using values of type 'char' to subscript
-Wformat - check printf et al format strings against args
-Wmain - type of main() is suspicious
-Wreturn-type - non-void function 'return;'; no function type


The 'usually useful' are those that are on by default in production
code but could be turned off during development or sometimes on a
case-by-case basis for production compiles in unusual circumstances.
An example is the setting for the -Wtrigraphs condition - usually code
should have no trigraphs but sometimes it needs to, and -Wtrigraphs
needs to be disabled in those cases. The usually useful ones are:

-Wimplicit - decl w/o type, or function call w/o decl
-Wcomment - when /* appears in a comment string (& more)
-Wmissing-braces - initializers not fully bracketed
-Wswitch - switch( ENUM ) that doesn't cover all cases
-Wtrigraphs - if any trigraphs are encountered
-Wunused - unused variable, value, label, static function
-Wunused-parameter - unused parameter
-Wuninitialized - variable might be used without having been set


The 'probably useful' are those that most likely in one of the
previous two categories but I don't have enough specific experience
with them to give an opinion as to which. Operationally they are
treated just as the 'usually useful' ones are. The probably useful
ones are:

-Wnonnull - NULL arg to "nonnull"-attributed parameters
-Wsequence-point - possible violations of sequence point rules
-Wswitch-enum - switch( ENUM ) cases != ENUM values
-Wunknown-pragmas - unrecognized #pragma
-Wstrict-aliasing - code that may break with -fstrict-aliasing


The 'objectionable' are those that might get used as diagnostic tools
from time to time but are turned off during all regular compiles. The
objectionable ones are:

-Wparentheses - eg, if(x=a+b). also, some unpaired if's
-Wswitch-default - switch() w/o 'default:' case


Discussion for -Wswitch-default:

-Wswitch-default
Warn whenever a "switch" statement does not have a
"default" case.

First of all, either -Wswitch or -Wswitch-enum seems like a more
accurate diagnostic tool when the switch() expression is of an
enumerated type [2].

Second, the decision to put in a 'default:' case is a programming
practice that is appropriate in some circumstances but not in others.
Apparently there is no way to locally override the warning for
switch() statements where the 'default:' is judged better left out.

Third, automatically adding a 'default:' to every switch() can reduce
the value of -Wswitch/-Wswitch-enum. It can be useful to leave off
the 'default:' so that when additional values are added to an
enumerated type then switch() statements on values of that type will
be flagged. Including -Wswitch-default will thwart this programming
practice.


Discussion for -Wparentheses:

-Wparentheses
Warn if parentheses are omitted in certain contexts,
such as when there is an assignment in a context where
a truth value is expected, or when operators are
nested whose precedence people often get confused
about.

Also warn about constructions where there may be con-
fusion to which "if" statement an "else" branch
belongs. [Example/explanation omitted.]

First, this warning covers several rather different conditions lumped
together under a single name. The different conditions should have
been broken out under separately invokable switches.

Second, the several conditions are essentially different because they
protect against different kinds of error causes. The reason for
writing 'if((x=a+b))' rather than 'if(x=a+b)' is to show that it
wasn't a mistype for 'if(x==a+b)'. The case with extra parentheses
inside expressions is different - here the concern is not that
mistyping might have occurred but that a non-expert reader might be
confused about precedence. (Curiously, the 'if((x=a+b))' form is
"more clear" only *after* reaching a certain level of experience.)
The case with braces for unpaired if's is different in still another
way - here the concern is any reader might be mislead (possibly after
future editing) by other formatting cues to mis-perceive the actual
structure. ([3]) They do have something in common: to some
extent they all reflect "style choices".

Third, as it happens the style choice about bracing to help with
unpaired if's is one I agree with. I expect I would set this warning
condition (as a 'usually useful' one) if it were separately available.

Fourth, the style choice to add redundant parentheses in certain
circumstances is one I disagree with. To say that another way, I
might choose to add redundant parentheses under some circumstances,
but the developer should make the choice about when, not the compiler.

Fifth, encouraging putting in redundant parentheses sends the wrong
message. In effect, we're telling developers that they needn't bother
learning the different levels of precedence because no one else knows
them either. This (probably unintended) side-effect of -Wparentheses
is especially harmful in novices, affected at a time when they are
most impressionable.

Sixth, adding redundant parentheses to correct "mistakes" flagged
under -Wparentheses develops bad programming habits. Clearly the code
'if((x=a+b))' does exactly the same thing as 'if(x=a+b)', but we have
to put in the extra parentheses "to shut up the compiler". So the
presence of -Wparentheses messages dilutes the effect of other warning
conditions, which are more likely to require code changes that
are substantive rather than just cosmetic.


Finally, a comment about -Wall generally and not about any specific
flag -W<xxx> on the list.

The set of -W<xxx> in -Wall tends to grow over time. This is
important because code that compiles today (or, on platform X) might
not compile tomorrow (or on platform Y). Because I work on source
that is distributed in bundles that include warning flag settings for
compilation directives (usually in makefiles), I've learned to avoid
using -Wall since its meaning in the target environment might very
well be different than the meaning of -Wall in the originating
environment. To a certain extent the same thing is true of individual
warning condition sets, but these happen on a smaller scale. The
set of conditions flagged by -Wall can increase drastically.

What's the word for this again? Oh yes, it's "innovation".


So there's my $64 answer. (Braces himself for the rebuttals
sure to follow...)


Notes:

[1] To be more precise, I normally try to act as though -Werror
were set whether it actually is or not.

[2] Whoever is writing gcc documentation should consult a dictionary
more often; there is no such word as "enumeral". (Authority:
several unabridged dictionaries.)

[3] Personally I think any developer who doesn't use braces in every
case where controlled statements are not on the same line as their
control keyword is at least mildly insane. But I digress.

 

[Tim Rentsch]

On Tue, 14 Dec 2004 21:06:09 +0100, Charlie Gordon wrote:

...


True, except when the code is clearly correct. Having to compromise your
progrmming style due to dumb compiler diagnostics is a problem.

Whether the warning is dumb depends on what you think the compiler is
diagnosing. I consider it poor style to compare a signed value and an
unsigned value, both because the results can be surprising and because
it isn't always obvious when/where it happens. That the code happens
to produce the results I expect (and, since the 'for' loop has an
empty body, how could it not?) does not effect the presence of the
warning condition.

Related question - if the code were

int main(){
int i = 0;

if( 0 ){
i += i++;
}
return 0;
}

would you want no diagnostic on the 'i += i++;' line, since
after all it won't be executed?

 

[Tim Rentsch]

On Wed, 15 Dec 2004 21:37:13 -0800, James Dennett wrote:

...


That depends on whether you consider cluttered to be a better style than
keeping it simple. There is a legitimate issue that the compiler is
warning about, but it doesn't occur in this case and it is easy to see
that.

Your perception of what the issue is may not match the perceptions
of others; apparently doesn't for some people.

On the cluttered/simple comment - that depends on the metrics being
used to measure "cluttered" and "simple", and on the relative weights
of those metrics compared to other factors (such as "may cause
maintenance problems in the future"). It also depends on what you
suppose the resolution of the problem is; the code

int main(){
return 0;
}

removes the warning and also is simpler and less likely to cause
maintenance problems in the future (at least by my metrics) as
compared with the original.

 

[Charlie Gordon]

It is a normal, well known and portable idiom for getting the largest
value an unsigned integer type can represent. In this case the loop
condition also works if the type happens to be signed. You could hide the
details behind a macro if you wanted but I don't really see the need.

Not at all !
It is not well known, and it is not portable.
Furthermore, it is not reliable : it is blatant undefined behaviour (because of
integer overflow).
(size_t)-1 is not necessarily the largest value size_t can represent : just
think of the different representations C99 allows for integer arithmetic. It
may work for 2's complement, doesn't work for 1's complement (off by one), and
completely wrong for sign/magnitude.
This loop is horrible, it should be banned. I rest my case.

 

[Francis Glassborow]

Not at all !
It is not well known, Cannot comment
and it is not portable.

It is about as portable as you can get.

Furthermore, it is not reliable : it is blatant undefined behaviour (because of
integer overflow).

size_t is required to be an unsigned type and there is no undefined
behaviour for overflow for unsigned integer types.

(size_t)-1 is not necessarily the largest value size_t can represent

It is required to be by the C and the C++ Standards. There is absolutely
no options.

: just
think of the different representations C99 allows for integer arithmetic.

Completely irrelevant, because those options are only applicable to
signed integer types and both C and C++ specify exactly how an out of
range value for an unsigned integer type must be converted into an in
range value.

It
may work for 2's complement, doesn't work for 1's complement (off by one), and
completely wrong for sign/magnitude.
This loop is horrible, it should be banned. I rest my case.

It would be a good idea to learn a little more C before condemining the
code of others.