[Freeciv-Dev] Re: (PR#7279) Macro optimizations

["Jason Short" <jshort@xxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >

Ross Wetmore wrote:
>
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >
>
>
>    int i = 0;
>    printf("%d, %d, %d", i++ i++ i++);
>
>    // f() depends on and/or changes global data
>    printf("%d, %d, %d", f(), f(), f());
>
> or something more complex along these lines would be the case. One is
> as likely to see 2 1 0 as 0 1 2 in a random compiler according to what
> order it chose to push arguments on the stack, and maybe what order it
> thought best to generate the data to push in more complex operations.

I still can't believe you are using this as an argument in favor of macros
over functions.  In the above situation you aren't guaranteed what order
the parameters will be evaluated in, and you know this.  You shouldn't be
writing this code.


By comparison the rather innocuous

  map_to_native_pos(&nat_x, &nat_y, map_x++, map_y++);

should work just fine according to the rules of C, but because
map_to_native_pos is coded as a macro it will give you hideously buggy
results.  Only if you look at the implementation of map_to_native_pos will
you know this - you can't determine it by looking at the interface, and it
is undocumented.

Nor is it possible to avoid the problem by rewriting the macro using
temporary variables.  We can't declare local variables within the macro
because we can't use a block in it.  We can't use global variables because
we run into the sequence point problem
(http://www.eskimo.com/~scs/C-faq/q3.8.html).  And we aren't going to use
an "environment" variable passed by the caller because this changes the
interface, requiring rediculously huge changes to the code  as well as an
unnecessary amount of work by each caller.

With any luck the compiler will give you a warning if you try to use this
line of code (the sequence point problem, again - although that's not the
most obvious bug).  Otherwise you have no way to know that your code has
no hope of succeeding.



My definition of best practices in this case would be:

Any macro that pretends to be a function must evaluate each of its
arguments exactly once.

jason