[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[avr-gcc-list] Re: compiler bug?

From: David Brown
Subject: [avr-gcc-list] Re: compiler bug?
Date: Tue, 10 Feb 2009 13:00:58 +0100
User-agent: Thunderbird (Windows/20081209)

Richard F wrote:

Call me a newbie, but is there anything wrong with the following statement?

rtc.time_element.tm_wday = (rtc.time_element.tm_wday < 6) ? rtc.time_element.tm_wday++ : 0;

I checked K&R and it looks OK to me.
In WINAVR 20081205 the expression rtc.tm_wday++ doesn't appear to be evaluated.
The assembler looks like:

rtc.time_element.tm_wday = (rtc.time_element.tm_wday < 6) ? rtc.time_element.tm_wday++ : 0;
 1ca:    80 91 77 01     lds    r24, 0x0177
 1ce:    86 30           cpi    r24, 0x06    ; 6
 1d0:    08 f0           brcs    .+2          ; 0x1d4 <rt_clock+0x42>
 1d2:    80 e0           ldi    r24, 0x00    ; 0
 1d4:    80 93 77 01     sts    0x0177, r24


The generated assembly is correct - your statement is wrong (as well as being amazingly bad style - mixing side effects, assignments and the conditional operator).

If tm_wday < 6, then you are asking the compiler to make a temporary copy of tm_wday, then increment it, then assign the temporary copy (pre-increment) to tm_wday. Clearly, the compiler doesn't have to bother with the increment.

My advice, whether you are a newbie or not, is to avoid the ?: operator unless you have very specific reasons for using it - an explicit if () {} else {} is almost always clearer. Only use the ?: if the code really is clearer and neater in that form.

And never use assignment operators (like ++ or +=) inside an assignment - your statement will probably be wrong, possibly undefined, and certainly confusing.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]