Strange - a simple assignment statement shows error in VC++ but worksin gcc !

[David Schwartz]

The "this" that I was referring to was the defect introduced either by
writing the incorrect cast or by changing the types of either x or y so
that the cast becomes incorrect. What is the "it" that you're suggesting
should be made more noticeable? What mechanism have you suggested to
make that defect more noticeable? It's less noticeable without the cast
only in the sense that it is non-existent without the cast, which
doesn't sound like a problem to me.

The defect is still there without the cast. The programmer did not
make sure the line of code is safe for the conversions it makes. If it
works, it only does so by accident. If he did make sure the line of
code was safe, it would work with or without the cast because the
right cast would be there.

If you change the type of a variable, you need to check every line of
code that accesses that variable. If you don't check a line and it
actually works, it does so by accident. That a technique makes some
code work by accident is not to its credit.

DS

 

[David Schwartz]

What's obvious depends upon the skill level of the author, and any
conclusions you should reach consider both the possibility of intent vs.
the possibility of a low skill level. In my experience, such code is
perpetrated mainly by people with sufficiently low skill level that they
did not find the consequences obvious; I don't think I've ever seen such
code written by people for the reason you've given. So you'll have to
forgive me if I favor the "low skill level" explanation when I see such
code.

Then you can't make any inferences about intent at all.

Again, that depends upon the skill level. I've seen people write code
who had a sufficiently low skill level that they wrote i++, expecting it
to behave as if it were written as "i+1" or "++i". Therefore, yes, I
would not automatically assume that i++ was intended just because it was
written.

Really, I'm not interested in coming up with coding rules and
procedures for imbeciles. I guess I should state that I'm talking
about rules and standards for a professional coding environment.

You can argue against any coding rule this way, and all it says is
that coding rules don't help imbeciles.

Correct. People can vouch. Code can contain comments that vouch. Plain
code only "does", it never "vouches". Any code, no matter how obvious
it's intent may seem, might have been written with a different intent in
mind - only examination of an explicit statement of intent (such as
design documentation) can justify reaching any conclusions about intent.

Okay, you write the rules for people who don't know how to code and
I'll talk about how people who know how to code *should* code, so that
others can review their code and they generate maintainable,
validatable, code.

DS

 

[Rainer Weikusat]

Then you can't make any inferences about intent at all.

'intent' is a transient state-of-mind of the person who wrote the code
and it is really impossible to make inferences about 'intent' from
'code', eg how can you know that this person wasn't intentionally
trying to write dysfunctional code[*]? Answer: You cannot. Fortunately,
the 'intent' of the code author still doesn't matter the least: What
matters is the content, this being the code itself, which is meant to
include 'functions performed by this code' and the differences (if any)
to 'functions that would need to be performed for the code to work'
(as required by specification ...).

[*] Or what about the 'intent' to get out of the office soon
in order to date $person_of_suitable_gender?

Besides, there are no 'obvious consequences' of any statement in any
artificial language. It's all arbitrary and exists only as
defined. Let's do a little thought experiment: Assume someone who is
unaware of the mathematical meaning of '+' but is aware that writing

"abd" + "def"

in C++ will yield the string "abcdef". Isn't it obvious from that
that writing

123 + 456

should yield the number 123456?

 

[jameskuyper]

The defect is still there without the cast. The programmer did not
make sure the line of code is safe for the conversions it makes.

You can't know what the programmer made sure of. Just because he
didn't insert a redundant cast doesn't prove that he ignored the
issue, any more than the fact that you would insert a redundant cast
proves that you payed attention to the issue.

 

[Keith Thompson]

Okay, you write the rules for people who don't know how to code and
I'll talk about how people who know how to code *should* code, so that
others can review their code and they generate maintainable,
validatable, code.

Ok, here are the rules I follow.

If a cast can be removed without changing the meaning of the code,
remove it (better, don't write it in the first place). Be susicious
of all casts. Never add a cast for the sole purpose of silencing a
warning. Never use a cast unless (a) it's actually necessary, or (b)
avoiding it would make the code needlessly complex. Examples of the
latter are conversions involving pointers (which tend to be
non-portable), arguments to variadic functions such as printf, and
complicated arithmetic expressions where you need better control over
the types of subexpressions than the language's default rules provide.
If an operand needs a cast, consider the possibility of making that
operand the expected type in the first place (this isn't always
possible and/or practical).

The point is that C's implicit conversions *usually* do the right
thing; casts are, in my opinion, only for cases where they don't.

In a sense, it's unfortunate that C's syntax doesn't distinguish
between, for example, an assignment with an implicit type conversion
and an assignment with no type conversion at all. But it also doesn't
distinguish between a correct cast and an incorrect cast, so I don't
think adding casts to replace implicit conversions is helpful. If I
see:
x = y;
I can't understand what it means unless I know the types of x and y,
and therefore whether there will be an implicit conversion. If I see
x = (type_of_x)y;
I'm really no better off; y could already be of type type_of_x, and x
might *not* be of type type_of_x; in the latter case, there's an
implicit conversion in addition to the explicit one.

These rules work quite well for me and, I believe, for a lot of other
programmers. Nobody is saying you have to follow them, but claiming
that your way is right *and there is no valid counterargument* is
nothing more than flamebait.

 

[jameskuyper]

Keith Thompson wrote:
....

warning. Never use a cast unless (a) it's actually necessary, or (b)
avoiding it would make the code needlessly complex. Examples of the
latter are conversions involving pointers (which tend to be
non-portable), arguments to variadic functions such as printf, and
complicated arithmetic expressions where you need better control over
the types of subexpressions than the language's default rules provide.

Did you mean "former" rather than "latter"? These all seem to be
examples of (a). I'd be interested in an example of (b).

 

[Keith Thompson]

Keith Thompson wrote:
...

Did you mean "former" rather than "latter"? These all seem to be
examples of (a). I'd be interested in an example of (b).

No, I meant "latter", but I was a little sloppy. Conversions
involving pointers typically do require casts, so those are examples
of (a). But the others are examples of (b).

For example, assuming that "%zu" is not available, this:

printf("sizeof(int) = %lu\n", (unsigned long)sizeof(int));

could be written as:

unsigned long sizeof_int = sizeof(int);
printf("sizeof(int) = %lu\n", sizeof_int);

The cast can be avoided by using a temporary object; the explicit
conversion of the cast is replaced by the implicit conversion in the
temporary object's initializer. But using the cast is simpler.

The same can apply to complicated arithmetic expressions.

 

[CBFalconer]

David Schwartz wrote:
.... snip ...


You can't know what the programmer made sure of. Just because he
didn't insert a redundant cast doesn't prove that he ignored the
issue, any more than the fact that you would insert a redundant
cast proves that you payed attention to the issue.

The point is that a cast removes the compiler warnings. You
probably have less complex methods of doing that and allowing
yourself, and all others, to ignore serious defects. It is
obviously better to have the program crash in execution, rather
than at compilation. It gives the users all that confidence "This
system detects problems".

Most code is improved by deleting all casts. Variadic functions
are a notable exception.

 

[CBFalconer]

.... snip ...

In any rational coding environment, programmers are assumed to
intend the obvious consequences of their code. Programmers are
assumed to vouch that the the obvious things they've done are
safe. They may be wrong about the consequences being safe or
what the consequences are -- but if they don't intend the
obvious consequences, they're simply so broken they aren't
doing what we call programming.

Loud disagreement. In any rational environment programmers are
warned when they write code that has unobvious consequences.
Unfortunately the organization of C is such that this detection is
often impossible.

 

[peter koch]

David Schwartz wrote: [snip]

So, a person who writes a cast to shut up a warning, without
understanding that the resulting conversion will have consequences he
wasn't aware of, can be said to have intended that those consequences occur?

I am stepping sideways into this discussion, so forgive me if I have
missed some of the finer points.
First, I must state that so far I am mostly on the side of David
Schwartz. If a compiler comes with a warning, we have to first decide
if that warning is or could be reasonable. If we find the warning
reasonable, we redesign the code in order to make the warnings
disappear, and if the warning is unreasonable, we find a way to
disable it (or throw the compiler away ;-)).
The principle is that our code should compile clean.

But I also have a very strong dislike for conversions, and one of the
rules - at least in C - should be that you do not have any conversion
that is not accompagnied by a comment. If this simple and good rule is
adhered to, none of the problems mentioned in this discussion would
surface.
[snip]

Every one of the cases I was referring to involved a cast written by a
programmer who had no idea that there was a conversion (or at least,
that it was a conversion with non-trivial consequences), and who
therefore had not in fact considered whether or not it was safe.

These problems would be determined during codereview, and the
programmer would learn that he would need to know his tools better
before writing any more code.

If someone signs a document he does not understand, and it contains
assertions that he would have disagreed with, had he understood them,
does this mean that he now agrees with those assertions? Writing his
signature on the document does not magically change his mind to match
the words of that document.

But he is not just signing the document - he has a lawyer (the code
reviewer) holding his hands. Even in places where you don't really
review code you should review code from the programmers that you do
not know.

Similarly, inserting a cast in code does not magically change the
developers intention to match what will actually happen. A convention of
inserting casts only when you are willing to vouch for their safety does
not magically bring people into conformance with that convention who
wrote their code without being aware of it, or without a desire to
conform to it. I do not defend the competence of these programmers; I
merely point out that they do exist, and code written by such people has
to be dealt with occasionally.
[snip]
In the typical data path for my programs, data is read from a file into
objects with typedefs associated with the HDF library like float32. It
is then passed to the SDP Toolkit library which requires that they have
   the type PGSt_double (it is not essential that you know what those
libraries are; it is sufficient that you understand that they are third
party libraries that define platform-dependent typedefs for the types
used in the library's interfaces). For some purposes, the data is passed
to C standard library functions which require that it be of type
'double'. When all processing is complete, the data is written to the
output file with the type float32 again.
Yuck!


That's a minimum of three casts for every piece of data flowing through
the program. One of the first things I did when I became responsible for
these programs was to rearrange it so that the minimum was actually
achieved; the original code performed at least twice the minimum number
of conversions. It's not uncommon for half the lines that refer to a
give piece of data to involve converting it from one type to another.
That's not true in the science code, but actual science code constitutes
only about 10% of my programs; the remaining 90% is involved with
pushing data around from one place to another.

I don't understand why all these standard conversions have to take
place using basic conversion functions. In your case (and being bound
to C), I would have small ideomatic functions do the conversion
instead - validating range in debug mode. That would result in only
six total conversions being performed.

/Peter

 

[Keith Thompson]

But I also have a very strong dislike for conversions, and one of the
rules - at least in C - should be that you do not have any conversion
that is not accompagnied by a comment. If this simple and good rule is
adhered to, none of the problems mentioned in this discussion would
surface.

[...]

Interesting idea.

So would you require a comment for each of the following?

float f = 1.0; /* conversion from double to float */
short s = 1; /* conversion from int to short */
int *p = NULL; /* conversion from ? to int* */

In the latter case, the type of NULL is likely to be int, but it could
be void* or even some integral type other than int.

And what about this?

printf("Hello, world\n");

Here the identifier printf is converted from a function type to a
pointer-to-function type (assuming printf isn't defined as a macro),
and the string literal "Hello, world\n" is converted from char[14] to
char*, and then from char* to const char*.

C is so full of implicit conversions that trying to comment all of
them is likely to result in more comments than code. On the other
hand, if you want to require comments for some subset of conversions,
that might be reasonable -- assuming you can define the subset clearly
enough.

(I'm assuming you really meant "conversions", not "casts".)

 

[CBFalconer]

.... snip ...

The project I work on is constrained by coding standards imposed
by our client which prohibit implicit conversions in delivered
code. In my experience with this prohibition, it makes the code
harder to read. It inhibits useful diagnostics that would have
otherwise have been generated when someone makes the mistake of
writing a cast that was intended to pointlessly duplicate an
implicit conversion to satisfy this prohibition, but which
actually performs a conversion that can only occur explicitly.
It makes casts ubiquitous, preventing them from serving as an
indication that something unusually dangerous has been attempted.

You have my sympathy. Have you tried to educate the client?

 

[David Schwartz]

Loud disagreement.

Good, which of the things I said do you disagree with?

 In any rational environment programmers are
warned when they write code that has unobvious consequences.

I never said anything about unobvious consequences.

Unfortunately the organization of C is such that this detection is
often impossible.

I never expressed an opinion on that.

If I say "two plus two is four", how does "three plus three is six"
constitute "loud disagreement"?

DS

 

[David Schwartz]

So would you require a comment for each of the following?

    float f = 1.0; /* conversion from double to float */

This is obviously safe, and it doesn't really particularly matter
whether the programmer realized there is conversion or not. I would
hope no rational compiler would issue a warning for this code except
in some crazy pedantic mode no sane person would use.

    short s = 1;   /* conversion from int to short    */

Same here. This is "obviously safe", obvious even to automated tools.

    int *p = NULL; /* conversion from ? to int*       */

In the latter case, the type of NULL is likely to be int, but it could
be void* or even some integral type other than int.

And what about this?

Again, same point. Obviously safe, no rational reviewer (human or
automated) would be concerned by these cases.

    printf("Hello, world\n");

Here the identifier printf is converted from a function type to a
pointer-to-function type (assuming printf isn't defined as a macro),
and the string literal "Hello, world\n" is converted from char[14] to
char*, and then from char* to const char*.

C is so full of implicit conversions that trying to comment all of
them is likely to result in more comments than code.  On the other
hand, if you want to require comments for some subset of conversions,
that might be reasonable -- assuming you can define the subset clearly
enough.

The subset is not perfectly precisely defined, IMO, but the following
considerations apply:

1) If it quiets a rational compiler warning, that weighs in favor of a
cast.

2) If the conversion is obviously safe, that weighs against a cast.

3) If the cast indicates the programmer was aware of an unobvious
conversion, that weighs in favor of the cast.

4) If the cast points to a reviewer that a conversion takes place,
that weighs in favor of the cast.

In most cases, these factors all turn into the same thing. If it's
obviously safe, there's nothing to know the programmer is aware of or
warn the reviewer about. Most compilers, even at high warning levels,
only issue warnings about conversion that are not obviously safe.

So gray areas are the exception rather than the rule.

GCC does issue some warnings in cases where, IMO, such a warning is
ridiculous. This makes a policy of "silence all compiler warnings by
vouching for the safety of the conversion" untenable.

An example would be (sorry, only have a C++ example handy, but the
concept is the same):

unsigned char uc_xor(unsigned char bar, unsigned char qux)
{
unsigned char ret=bar;
ret^=qux; // G++ 4.3.3 alerts here
return ret;
}

g++ 4.3.3 -Wconversion ->
"warning: conversion to 'unsigned char' from 'int' may alter its
value"

I don't know offhand of any cases where GCC, on C code, issues
obviously bogus warnings, but it wouldn't surprise me if there were
some. And I wouldn't suggest butchering your code to "fix" that.

(I'm assuming you really meant "conversions", not "casts".)

Definitely. A hard rule that all conversions must be commented or,
worse, identified by casts, is truly terrible. Anyone who actually had
such a rule would, we hope, at least have some implicit understanding
that it doesn't apply to trivial, obviously safe cases.

DS

 

[Keith Thompson]

Definitely. A hard rule that all conversions must be commented or,
worse, identified by casts, is truly terrible. Anyone who actually had
such a rule would, we hope, at least have some implicit understanding
that it doesn't apply to trivial, obviously safe cases.

Hmm. My remarks were addressed to peter koch, who wrote:
| But I also have a very strong dislike for conversions, and one of the
| rules - at least in C - should be that you do not have any conversion
| that is not accompagnied by a comment. If this simple and good rule is
| adhered to, none of the problems mentioned in this discussion would
| surface.

This is of course a public forum, and you're free to respond, but you
give the impression that you're speaking on his behalf. I don't see
how you can be sure of what he meant.

The idea that all casts should be accompanied by comments is an
interesting one (to me, at least).

 

[Keith Thompson]

The project I work on is constrained by coding standards imposed by
our client which prohibit implicit conversions in delivered code.

[...]

Have you explained to the client that almost every reference to a
function or array name involves an implicit conversion? There are
three implicit conversions in printf("Hello, world\n").

 

[James Kuyper]

This is obviously safe, and it doesn't really particularly matter
whether the programmer realized there is conversion or not. I would
hope no rational compiler would issue a warning for this code except
in some crazy pedantic mode no sane person would use.

I routinely use a compiler which has such a mode. In order to meet the
contractual requirement that I've mentioned earlier, I make a point of
compiling my code with that mode turned on at least once before each
delivery (it is far too much of an annoyance to use that mode any more
often than that).

I would avoid such warnings by modifying the above code to:

float f = 1.0F;

Making such modifications doesn't bother me. It's the cases where such
warnings can only be shut up by inserting a redundant cast that annoy me.

 

[James Kuyper]

jameskuyper wrote:
... snip ...

You have my sympathy. Have you tried to educate the client?

I did bring up that issue, along with several other issues regarding
those coding standards. I also mentioned adding C99 and C++ to the list
of permitted languages for delivered code. I think that the C++
suggestion is the only one of those that is likely to be approved.

 

[James Kuyper]

The project I work on is constrained by coding standards imposed by
our client which prohibit implicit conversions in delivered code.

[...]

Have you explained to the client that almost every reference to a
function or array name involves an implicit conversion? There are
three implicit conversions in printf("Hello, world\n").

No. I'll have to check the exact wording of the prohibition again; it
may be written to exclude those cases.

 

[peter koch]

[...]> But I also have a very strong dislike for conversions, and one of the

rules - at least in C - should be that you do not have any conversion
that is not accompagnied by a comment. If this simple and good rule is
adhered to, none of the problems mentioned in this discussion would
surface.

[...]

Interesting idea.

So would you require a comment for each of the following?

    float f = 1.0; /* conversion from double to float */
    short s = 1;   /* conversion from int to short    */
    int *p = NULL; /* conversion from ? to int*       */

In the latter case, the type of NULL is likely to be int, but it could
be void* or even some integral type other than int.

And what about this?

    printf("Hello, world\n");

Here the identifier printf is converted from a function type to a
pointer-to-function type (assuming printf isn't defined as a macro),
and the string literal "Hello, world\n" is converted from char[14] to
char*, and then from char* to const char*.

C is so full of implicit conversions that trying to comment all of
them is likely to result in more comments than code.  On the other
hand, if you want to require comments for some subset of conversions,
that might be reasonable -- assuming you can define the subset clearly
enough.

(I'm assuming you really meant "conversions", not "casts".)

Very nice one, but I kind of doubt that last statement. I naturally
meant explicit conversion, in C: cast.

/Peter