[Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxxx>]

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

On Mon, 30 Aug 2004, Jason Short wrote:

> <URL: http://rt.freeciv.org/Ticket/Display.html?id=9869 >
>
> > [glip - Mon Aug 30 17:06:31 2004]:
> >
> > On Sun, 29 Aug 2004, Jason Short wrote:
> >
> > > <URL: http://rt.freeciv.org/Ticket/Display.html?id=9869 >
> > >
> > > Gregory Berkolaiko wrote:
> > >
> > > > I am not sure which is the better way to fix this: do temporary
> > > > assignments inside index_to* functions or do it in the caller.
> > The first
> > > > way might give more work to compiler (optimizing), the second will
> > give
> > > > more work to us (reviewing all uses).
> > >
> > > We can't do temporary assignments unless we make it an inline
> > function.
> > >   Changing the users is fine but we also have to rename it as
> > > INDEX_TO_MAP_POS.  I would prefer the former but either is fine.
> >
> > I am confused.  First, why cannot we do temp assignments in a macro
> > (unless we want it as an lvalue or something)
> >
> > #define blah(a, b) {int c = a; b += c; b *= c;}
>
> You can, but then the macro cannot be used like a function; it cannot be
> used in an expression.  I had assumed that some users would do this but
> maybe they don't.
>
> > Second, if we change the users, why should we rename it? (Or is it
> > because
> > of the new guidelines macros vs inlines?)
>
> The guidelines.  We see in this case why they are needed - this macro
> looks like a function but it's not.  If it evaluates arguments more than
> once it should be CAPITALIZED.
>
> Note that a _lot_ of macros don't satisfy this requirement.
> native_to_map_pos and map_to_native_pos may be the most dangerous ones.
>
> > Third, which one you'd actually prefer, to make it inline?
>
> I prefer making it inline since its sister function map_pos_to_index is
> inline.  For the other conversion macros we should probably just
> capitalize them.
>
> > Fourth, why don't you make a patch ;)
>
> Attached.  Is this sufficient to fix the DEBUG problem?

I will run some tests now.  Should I also test the speed change due to
your patch or can we safely assume it's not worse?

G.