[Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
<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.
- [Freeciv-Dev] (PR#9869) Bug: DDEBUG changes savegames, Gregory Berkolaiko, 2004/08/29
- [Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames, Jason Short, 2004/08/29
- [Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames, Gregory Berkolaiko, 2004/08/30
- [Freeciv-Dev] (PR#9869) Bug: DDEBUG changes savegames, Jason Short, 2004/08/30
- [Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames,
Gregory Berkolaiko <=
- [Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames, Gregory Berkolaiko, 2004/08/30
- [Freeciv-Dev] (PR#9869) Bug: DDEBUG changes savegames, Gregory Berkolaiko, 2004/08/31
- [Freeciv-Dev] (PR#9869) Bug: DDEBUG changes savegames, Jason Short, 2004/08/31
- [Freeciv-Dev] Re: (PR#9869) Bug: DDEBUG changes savegames, Gregory Berkolaiko, 2004/08/31
|
|