[Freeciv-Dev] Re: [PATCH] more small directional cleanups

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 07:07 PM 01/08/20 -0700, Trent Piepho wrote:
>On Mon, 20 Aug 2001, Ross W. Wetmore wrote:
>> I think we are all agreed that that is about the only known use for it.
>> 
>> So you agree that a comment to stop people from using it where they
>> shouldn't is a reasonable thing to add, yes?
>
>I suppose that does make sense, I can see how something might think that
>if(!is_normal_tile(x,y)) normalize_map_pos(&x,&y) was a good idea when really
>the extra check serves no purpose.
>
>It's not entirely useless though.  normalize_map_pos takes pointer argument,
>so the compiler will have to move x and y out of registers and into memory. 
>If most positions don't need to be normalized, then checking for normality
>(normalness?  normalizedness?) using inlined code, like is_normal_tile()
could
>be, would just be a few machine instructions that would save a function call
>and some register saving.  But I'm just grasping at straws here...

I think so. Most compilers will only store their register contents back to
memory when they need to. As an inline or macro function, this all comes
out in the wash. And if you use the function the extra noise won't be
noticeable in all the other stack frame building updates.

There is a nitpick in that the programmer has to setup the local storage
variables to use noramlize_map_pos, but the same applies to your line
above.

The real problem with normalize_map_pos is that it returns an is_valid
status, whereas you often want just the transformed coordinate like in
map_adjust_x() for use in an expression. But it can't return 2 coordinates
and we should probably be moving away from a model where each coordinate
can be thought of as being updated independently, and to a model where
the actual use is done inside a checked if clause.

Cheers,
RossW
=====