Frederick Gotham wrote:
Here's a sample function which converts a string to all uppercase:
#include <cassert>
#include <cctype>
void StringUp( char *p )
{
do assert( *p >= 0 );
while( *p = std::toupper( *p ), *p++ );
}
Would the "Sequence point rule" be violated if the code were changed to
the following:
#include <cassert>
#include <cctype>
void StringUp( char *p )
{
do assert( *p >= 0 );
while( *p++ = std::toupper(*p) );
}
a) Feels like it: you dereference the same pointer twice within an
expression that, as a side effect, changes the pointer. Sounds dangerous.
Now, suppose that carefull analysis of the C++ standard showed the code in
question to be legit. Would you then consider it good code? Would you be
inclined to put that analysis as a comment next to the code so that a
maintainer does not feel the need to check carefully?
b) Why not avoid borderline cases and comma operator trickery altogether.
Why not put every action on a line of its own:
#include <cctype>
void StringUp( char * p ) {
while ( *p != 0 ) {
*p = std::toupper( *p );
++ p;
}
}
c) I do not understand the assert( *p >= 0 ) in your code. If char is
unsigned, it will never fail, if char is signed, it may. However there is
no reason why StringUp() should not be called on strings containing
negative characters. I cannot see a reason to make that restriction part of
the contract.
Best
Kai-Uwe Bux