## Problem with macro

11 posts / 0 new
Author
Message

Hi, having a little problem with a macro and a ternary operator

The code below

```#define LED_PIN PORTC4
#define LED_ON  PORTC |=  (1 << LED_PIN)
#define LED_OFF PORTC &= ~(1 << LED_PIN) // <- lvalue required as left operand of assignment

void main()
{
uint8_t b = 0xAA;
(b & 0x01) ? LED_ON : LED_OFF; // <- in expansion of macro LED_OFF

if(b & 0x01)
LED_ON;
else
LED_OFF;
}```

Is giving the compile errors shown in the comments.

I am not overly worried about it, as I have heard that the if/else construct may actually result in quicker code than the ternary operator,

But I would love to know why it doesn't compile.

kcr

This topic has a solution.
Last Edited: Fri. Dec 1, 2017 - 01:55 PM
This reply has been marked as the solution.

Do the macro expansion by hand, and then have a closer look at the result having "operator precedence" in mind.

Stefan Ernst

Ahhh!. OK, Got it. Thanks

Using -save-temps the .i file looks like:

```# 7 "avr.c"
void main()
{
uint8_t b = 0xAA;
(b & 0x01) ?
# 10 "avr.c" 3
(*(volatile uint8_t *)((0x15) + 0x20))
# 10 "avr.c"
|= (1 <<
# 10 "avr.c" 3
4
# 10 "avr.c"
) :
# 10 "avr.c" 3
(*(volatile uint8_t *)((0x15) + 0x20))
# 10 "avr.c"
&= ~(1 <<
# 10 "avr.c" 3
4
# 10 "avr.c"
);

if(b & 0x01)

# 13 "avr.c" 3
(*(volatile uint8_t *)((0x15) + 0x20))
# 13 "avr.c"
|= (1 <<
# 13 "avr.c" 3
4
# 13 "avr.c"
);
else

# 15 "avr.c" 3
(*(volatile uint8_t *)((0x15) + 0x20))
# 15 "avr.c"
&= ~(1 <<
# 15 "avr.c" 3
4
# 15 "avr.c"
);
}```

But that's quite tricky to follow. Maybe easier as:

```void main()
{
uint8_t b = 0xAA;
(b & 0x01) ?
(*(volatile uint8_t *)((0x15) + 0x20))
|= (1 <<
4
) :
(*(volatile uint8_t *)((0x15) + 0x20))
&= ~(1 <<
4
);

if(b & 0x01)

(*(volatile uint8_t *)((0x15) + 0x20))
|= (1 <<
4
);
else

(*(volatile uint8_t *)((0x15) + 0x20))
&= ~(1 <<
4
);
}```

and clearer still as:

```void main()
{
uint8_t b = 0xAA;
(b & 0x01) ? (*(volatile uint8_t *)((0x15) + 0x20)) |= (1 << 4)
: (*(volatile uint8_t *)((0x15) + 0x20)) &= ~(1 << 4);

if(b & 0x01)
(*(volatile uint8_t *)((0x15) + 0x20)) |= (1 << 4 );
else
(*(volatile uint8_t *)((0x15) + 0x20)) &= ~(1 << 4 );
}```

kcrelectronics wrote:
the if/else construct

Try to get into the habit of making your macros "code safe" -- no matter where you place them in source text, there won't be unintended consequences.

In very general terms, wrap them in parentheses.

kcrelectronics wrote:
if(b & 0x01) LED_ON; else LED_OFF;

Many of us will always make a block for if/else even if only one apparent statement...

if (xxx)

{

stuff;

}

else

{

otherstuff;

}

The reasoning is twofold:  It helps to guard against forgetting to add the block when adding more stuff.  And it helps to protect against macro expansion as in your case that might have unintended consequences.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

kcrelectronics wrote:
the if/else construct may actually result in quicker code than the ternary operator

Really?

Top Tips:

1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

Side note, to despise if you want to ;-) ...

I sort of cringe when the ternary operator is used for purposes other than to produce a value (e.g to produce a value for an assignment). I.e. this is where I think the ternary operator makes sense:

`variable = condition ? expression1 : expression2;`

The cringe comes when the ternary operator is used for controlling program flow like in

`condition ? statement1 : statement2;`

For the latter I always(?) will go for the, in my mind, much clearer

```if (condition)
statement1;
else
statement2;```

And, yes.. I always make those statements blocks so that will actually be

```if (condition) {
statement1;
} else {
statement2;
}
```

Dislike the indentation style at you discretion - that's not what I'm shooting at here.

My argument has nothing do do with efficiency of the code in terms of size or execution speed, since

i) for (generally) most of the code you write the efficiency loss is negligible, and

ii) for the small fraction of the code where efficiency is critical you might very well go for a more predictable optimization technique (e.g. assembler).

It has all to do with readability and fitness for debugging and maintenance. This applies to 100% of the code you write.

You're free to disagree :-)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Totally agree.

The one proviso I'd make is that as well as value assignment I have often used ternary for the likes of:

```void fn(bool state) {
printf("Switch is %s\n", state ? "ON" : "OFF");
}```

which isn't exactly assignment - but close.

JohanEkdahl wrote:
The cringe comes when the ternary operator is used for controlling program flow like in

`condition ? statement1 : statement2;`

I would agree that this is absolutely cringeworthy!

Why would one do that? What is the perceived/alleged benefit?

Top Tips:

1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

clawson wrote:
I have often used ternary for the likes of:

I don't know whether "often", but when building a e.g. screen line it can be helpful to do a range check on the parameter and provide a default "n/a" or similar.

awneil wrote:
kcrelectronics wrote: the if/else construct may actually result in quicker code than the ternary operator Really?

"It depends" on the context, but indeed sometimes in CV the straightforward if/else is indeed a bit smaller/faster.  [now I need to try to dig out the apps where I tested it...]

The below repeated 16 times...

```	if (valve_mask & VALVE_1)
{
OUT_1 = 1;
}
else
{
OUT_1 = 0;
}

```

The temptation is to use

```	OUT_1 = (valve_mask & VALVE_1) ? 1 : 0;
```

...as it appears "cleaner".  But:

```                 ; 0000 0349 	if (valve_mask & VALVE_1)
0000e5 fe40      	SBRS R4,0
0000e6 c002      	RJMP _0x1D
; 0000 034A 		{
; 0000 034B 		OUT_1 = 1;
0000e7 9a42      	SBI  0x8,2
; 0000 034C 		}
; 0000 034D 	else
0000e8 c001      	RJMP _0x20
_0x1D:
; 0000 034E 		{
; 0000 034F 		OUT_1 = 0;
0000e9 9842      	CBI  0x8,2
; 0000 0350 		}
_0x20:
; 0000 0351
; 0000 0352 	OUT_1 = (valve_mask & VALVE_1) ? 1 : 0;
0000ea fe40      	SBRS R4,0
0000eb c002      	RJMP _0x23
0000ec e0e1      	LDI  R30,LOW(1)
0000ed c001      	RJMP _0x24
_0x23:
0000ee e0e0      	LDI  R30,LOW(0)
_0x24:
0000ef 30e0      	CPI  R30,0
0000f0 f411      	BRNE _0x26
0000f1 9842      	CBI  0x8,2
0000f2 c001      	RJMP _0x27
_0x26:
0000f3 9a42      	SBI  0x8,2
_0x27:
```

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Fri. Dec 1, 2017 - 04:04 PM

clawson wrote:
Totally agree.

Ditto - see #9

The one proviso I'd make is that as well as value assignment I have often used ternary for the likes of:

```void fn(bool state) {
printf("Switch is %s\n", state ? "ON" : "OFF");
}```

which isn't exactly assignment - but close.

Yes, I think that's acceptable.

Maybe the rule for Acceptable Use could be stated as, "where the purpose is to obtain a value" ?

Although it does somewhat compromise the "fitness for debugging".

JohanEkdahl wrote:
Dislike the indentation style

OK - I do.

Top Tips:

1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...