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();
}
Index: src/ops/object.ops
===================================================================
--- src/ops/object.ops (revision 17785)
+++ src/ops/object.ops (working copy)
@@ -60,7 +60,7 @@
"Method '%Ss' not found", meth);
}
interp->current_object = object;
- interp->current_cont = NEED_CONTINUATION;
+ interp->current_cont = (PMC *)NEED_CONTINUATION;
dest = (opcode_t *)VTABLE_invoke(interp, method_pmc, next);
goto ADDRESS(dest);
}
@@ -76,7 +76,7 @@
next = expr NEXT();
interp->current_object = object;
- interp->current_cont = NEED_CONTINUATION;
+ interp->current_cont = (PMC *)NEED_CONTINUATION;
dest = (opcode_t *)VTABLE_invoke(interp, method_pmc, next);
goto ADDRESS(dest);
}
Index: src/ops/io.ops
===================================================================
--- src/ops/io.ops (revision 17785)
+++ src/ops/io.ops (working copy)
@@ -372,7 +372,7 @@
if (pio->vtable->base_type != enum_class_ParrotIO)
real_exception(interp, NULL, PIO_ERROR,
"Cannot read line from empty filehandle");
- io = PMC_data(pio);
+ io = (ParrotIO *)PMC_data(pio);
if (!io)
real_exception(interp, NULL, PIO_ERROR,
"Cannot read line from empty filehandle");
Index: src/ops/sys.ops
===================================================================
--- src/ops/sys.ops (revision 17785)
+++ src/ops/sys.ops (working copy)
@@ -297,7 +297,7 @@
real_exception(interp, next, NEG_SLEEP,
"Cannot go back in time");
}
- next = Parrot_sleep_on_event(interp, (FLOATVAL) $1, next);
+ next = (opcode_t *)Parrot_sleep_on_event(interp, (FLOATVAL) $1, next);
goto ADDRESS(next);
}
@@ -307,7 +307,7 @@
real_exception(interp, next, NEG_SLEEP,
"Cannot go back in time");
}
- next = Parrot_sleep_on_event(interp, $1, next);
+ next = (opcode_t *)Parrot_sleep_on_event(interp, $1, next);
goto ADDRESS(next);
}
Index: src/charset.c
===================================================================
--- src/charset.c (revision 17785)
+++ src/charset.c (working copy)
@@ -62,7 +62,7 @@
CHARSET *
Parrot_new_charset(Interp *interp)
{
- return mem_sys_allocate(sizeof (CHARSET));
+ return (CHARSET *)mem_sys_allocate(sizeof (CHARSET));
}
void
@@ -184,9 +184,9 @@
* loading of charsets from inside threads
*/
if (!n)
- all_charsets->set = mem_sys_allocate(sizeof (One_charset));
+ all_charsets->set = (One_charset *)mem_sys_allocate(sizeof (One_charset));
else
- all_charsets->set = mem_sys_realloc(all_charsets->set, (n + 1) *
+ all_charsets->set = (One_charset *)mem_sys_realloc(all_charsets->set, (n + 1) *
sizeof (One_charset));
all_charsets->n_charsets++;
all_charsets->set[n].charset = charset;
@@ -219,7 +219,7 @@
CHARSET *charset)
{
if (!all_charsets) {
- all_charsets = mem_sys_allocate(sizeof (All_charsets));
+ all_charsets = (All_charsets *)mem_sys_allocate(sizeof (All_charsets)); all_charsets->n_charsets = 0;
all_charsets->set = NULL;
}
@@ -320,11 +320,11 @@
nc = left->n_converters++;
if (nc) {
- left->to_converters = mem_sys_realloc(left->to_converters,
- sizeof (To_converter) * (nc + 1));
+ left->to_converters = (To_converter *)mem_sys_realloc(
+ left->to_converters, sizeof (To_converter) * (nc + 1)); }
else
- left->to_converters = mem_sys_allocate(sizeof (To_converter));
+ left->to_converters = (To_converter *)mem_sys_allocate(sizeof (To_converter));
left->to_converters[nc].to = rhs;
left->to_converters[nc].func = func;
}
Index: src/exec.c
===================================================================
--- src/exec.c (revision 17785)
+++ src/exec.c (working copy)
@@ -69,7 +69,7 @@
extern PARROT_API int Parrot_exec_rel_count;
Parrot_exec_objfile_t * const obj =
- mem_sys_allocate_zeroed(sizeof (Parrot_exec_objfile_t));
+ (Parrot_exec_objfile_t *)mem_sys_allocate_zeroed(sizeof (Parrot_exec_objfile_t));
exec_init(obj);
Parrot_exec_rel_addr = (char **)mem_sys_allocate_zeroed(4 * sizeof (char *));
obj->bytecode_header_size =
@@ -206,7 +206,7 @@
Parrot_exec_symbol_t *new_symbol;
symbol_number = obj->symbol_count;
- new_symbol = mem_sys_realloc(obj->symbol_table,
+ new_symbol = (Parrot_exec_symbol_t *)mem_sys_realloc(obj->symbol_table, (size_t)(obj->symbol_count + 1) * sizeof (Parrot_exec_symbol_t));
obj->symbol_table = new_symbol;
@@ -266,7 +266,7 @@
extern PARROT_API char **Parrot_exec_rel_addr;
extern PARROT_API int Parrot_exec_rel_count;
- new_relloc = mem_sys_realloc(obj->text_rellocation_table,
+ new_relloc = (Parrot_exec_rellocation_t *)mem_sys_realloc(obj->text_rellocation_table,
(size_t)(obj->text_rellocation_count + 1)
* sizeof (Parrot_exec_rellocation_t));
obj->text_rellocation_table = new_relloc;
could (PMC *) get added to the NEED_CONTINUATION macro
Index: include/parrot/sub.h
===================================================================
--- include/parrot/sub.h (revision 17785)
+++ include/parrot/sub.h (working copy)
@@ -109,7 +109,7 @@
* a flag to signal a Sub that a new RetContinuation should be created
*/
-#define NEED_CONTINUATION ((void*)1)
+#define NEED_CONTINUATION ((PMC*)(void*)1)
/*
* Sub and Closure share a Parrot_sub structure.
Kevin
> # 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?
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.
--
Andy Dougherty doug...@lafayette.edu
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 :)
> I think you'll find a lot of lower-level cleanups are in order.
[other stuff deleted.]
Oops! My previous message was not appropriate, and I didn't mean to send
it. Please ignore it. I'm sorry about that.
--
Andy Dougherty doug...@lafayette.edu
> On Tue Mar 27 10:54:17 2007, doughera wrote:
> > 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.
I think you'll find a lot of lower-level cleanups are in order. Leo
tended to assume all pointers could equally well point anywhere, and often
ignored alignment issues. He often declared stuff (void *) when it really
should have been something more specific. Unfortunately, he also tended
to reject patches to try to clean it up. I wish you better luck.
--
Andy Dougherty doug...@lafayette.edu
True. But while invoke() et al receive and return an (opcode_t*) the actual
running runloop (with e.g. predereferenced runcores) wants a void**, which
should OTOH not need a type cast.
E.g. in src/ops/core_ops_cgp.c
/* run_core_func_decl - tools/build/ops2c.pl -> Parrot::OpTrans::CPrederef */
void ** cgp_core(void **cur_op, Parrot_Interp interp)
The usage of properly declared void** op_ptrs inside that shouldn't trigger
much need for casting as opcode_t*.
leo
Nope. Sorry.
leo
It looks to me that I've already replied to that cancelled message. Therefore
another reply is maybe in order, which is a minimal summary of a quick grep
of old mails:
Date: Thu, 02 Jun 2005 23:18:56 +0200
---
Andy Dougherty wrote:
> Here's the Solaris patch:
BTW it seems that you don't have ci privs for Parrot, which IMHO is just
a blunder.
If you like to commit that stuff yourself please send me a mail with
your auth.perl.org account name and I'll forward it to Robert.
Thanks,
leo
---
Subject: svn access (was: [perl #38576] [PATCH] Make DETAIL_MEMORY_DEBUG
work. )
On Feb 15, 2006, at 18:53, Andy Dougherty (via RT) wrote:
> # New Ticket Created by Andy Dougherty
Andy, I've already asked once: don't you have svn access? If no (and if
you want it) please mail me your auth.perl.org account data, to get you
svn priv bits.
If yes, I'd really prefer that such patches just go in. And honestly
all your patches are well-thought and hight quality.
Thanks,
leo
Of course, some well defined macros could assist in cleaning this up.
For example...
#define PARROT_MEM_ALLOCATE(type) \
(type *)mem_sys_allocate(sizeof(type))
I don't know the Parrot opinion of macros, but it would certainly make
the code cleanup much easier.
> Of course, some well defined macros could assist in cleaning this up.
> For example...
>
> #define PARROT_MEM_ALLOCATE(type) \
> (type *)mem_sys_allocate(sizeof(type))
>
> I don't know the Parrot opinion of macros, but it would certainly make
> the code cleanup much easier.
That wouldn't hurt my feelings. It really wouldn't hurt my feelings to use a
macro for PARROT_MEM_FREE either; then it would be trivial to switch to a
memory poisoning scheme that hopefully generates segfaults much, much
earlier.
-- c