Re: MERGE: bsescm2cxx


On Sat, May 14, 2011 at 12:56:25AM +0200, Tim Janik wrote:
> On 13.05.2011 17:11, Stefan Westerfeld wrote:
> >>* For sfi_glue_gc_add(), please investigate if changing
> >>   the second argument to (void(*free_func)(void*)) is likely to
> >>   reduce casts. The reason the second funciton pointer argument is
> >>   just void* is convenience in C, but were're moving away from that
> >>   so it doesn't make sense to keep convenince C stuff if it makes
> >>   our C++ life harder.
> >
> >I can reduce the number of casts in shell/bsescm* by changing the signature,
> >however there is quite a bit of C code that either will not compile at all or
> >will compile with warnings if I do this (for instance in sfi/sfiglue.c or the
> >generated beast-gtk/bstgenapi.c), so overall a lot of new casts need to be
> >added elsewhere. I'd suggest to delay this change until more of the remaining
> >code is already C++ified.
> Hm, if we go like this then we end up with lots of C++-ified code that
> needs fixing after we've turned everything into C++. I.e. essentially
> demanding a 2 phase approach, which I'd like to avoid since it could
> allmost double our work. We should avoid any approach that forces us to
> introduce either a lot of C or a lot of useless C++ casts. One way
> is to port sfiglue along with the code you did. Another is the
> following, if something is just an issue on the language layer (i.e.
> not affecting ABI or semantics), I'd rather see us special case C++
> vs. C in some very few selected places, that allow easy cleanups.
> E.g.:
> #ifdef __cplusplus
> void sfi_glue_gc_add (void(*)(void*));
> #else /* !__cplusplus */
> void sfi_glue_gc_add (void*);	// FIXME: remove C-legacy
> #endif /* __cplusplus */
> This is a very minor thing to fix after the move (and it works
> because global extern "C" functions in C++ have the same symbol
> names as C functions) *and* it's easy to spot by later grepping for
> "FIXME",
> possibly even "C-legacy".

Your proposal sounds good, I've committed changes to the bsescm2cxx branch
which do this. This allows me to remove 5 casts from bsescm*, while I don't
need to change the C sources that rely on the old prototype.

   Cu... Stefan
Stefan Westerfeld, Hamburg/Germany,

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]