On Tue Mar 27 10:54:17 2007, doughera wrote:
> On Tue, 27 Mar 2007, Steve Peters wrote:
>
> > # New Ticket Created by Steve Peters
> > # Please include the string: [perl #42151]
> > # in the subject line of all future correspondence about this issue.
> > # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=42151 >
>
> Thanks for taking on this Herculean task. However, one thought immediately
> came to mind:
>
> >
> > Index: src/ops/experimental.ops
> >
=========================================================
==========
> > --- src/ops/experimental.ops (revision 17785)
> > +++ src/ops/experimental.ops (working copy)
> > @@ -520,7 +520,7 @@
> > Interp * const new_interp = (Interp *)PMC_data($1);
> > opcode_t *pc;
> > Interp_flags_SET(new_interp, PARROT_EXTERN_CODE_FLAG);
> > - pc = VTABLE_invoke(new_interp, $2, NULL);
> > + pc = (opcode_t *)VTABLE_invoke(new_interp, $2, NULL);
> > Parrot_runops_fromc_args(new_interp, $2, "P");
> > goto NEXT();
> > }
>
> Instead of sprinking (opcode_t *) casts everywhere, wouldn't it be better
> to declare the invoke() function as returning an (opcode_t *) ?
> Similarly for most of the other bits you patched. Wouldn't it make more
> sense in the long run to change the functions to say they return what they
> actually return?
>
I'll have to look into some of these a little more closely as I go through. I
should probably go
through and refactor as I work on this cleanup. I probably should reject this
particular patch
and look for some lower level cleanups that might make this whole process much
easier, such
as what you two have pointed out.
> Of course you can't always do that:
>
> > Parrot_new_charset(Interp *interp)
> > {
> > - return mem_sys_allocate(sizeof (CHARSET));
> > + return (CHARSET *)mem_sys_allocate(sizeof (CHARSET));
> > }
>
> mem_sys_allocate() obviously can only return one type, but is used in many
> contexts.
>
Yes, no getting around these ones :)
