On Thu, 24 Apr 2008, Andy Dougherty wrote:
> On Wed, 23 Apr 2008, Andy Dougherty wrote:
>
> > 2. There are some casting and type-punning warnings that have, as their
> > ultimate cause, the STACK_DATAP() macro. Getting rid of the
> > type-punning warning gives rise to a cast alignment warning.
> >
> > Looking up a level, the only uses for that macro are to return
> > Stack_Entry_t pointers. Why not be explicit about it in the definition
> > in include/parrot/stacks.h? The attached patch tries to do just that.
> > I haven't tested this, but it looks like it ought to work.
>
> Oops. It won't work because I missed a level of indirection.
Ok. Fixed. This version avoids both type-punning and cast alignment
warnings by declaring the 'data' element of a Stack_Chunk_t to be
of type Stack_Entry_t, since that's the only way it is ever used,
at least as far as I could tell.
> Oops. I can only say I was fooled by the function signatures that these
> functions returned 'void *', when they actually return 'void **'.
Now they return Stack_Entry_t *, since their return values always were
cast to that type anyway.
I have tested this on Solaris/SPARC with Sun's cc. It introduces no
new warnings or failures that I can detect. (The build actually doesn't
complete either with or without this patch, but it does get far enough
that I can run most of the core tests.)
diff -r -u parrot-x64-before/include/parrot/stacks.h
parrot-x64/include/parrot/stacks.h
--- parrot-x64-before/include/parrot/stacks.h 2008-04-23 13:58:26.000000000
-0400
+++ parrot-x64/include/parrot/stacks.h 2008-04-25 10:12:42.000000000 -0400
@@ -33,7 +33,7 @@
Parrot_UInt refcount;
union { /* force appropriate alignment of 'data'. If alignment
is necessary, assume double is good enough. 27-04-2007. */
- void *data;
+ Stack_Entry_t data;
#if PARROT_PTR_ALIGNMENT > 1
double d_dummy;
#endif
@@ -158,7 +158,7 @@
PARROT_API
PARROT_WARN_UNUSED_RESULT
PARROT_CANNOT_RETURN_NULL
-void* stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p))
+Stack_Entry_t* stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t
**stack_p))
__attribute__nonnull__(1)
__attribute__nonnull__(2)
FUNC_MODIFIES(*stack_p);
@@ -166,7 +166,7 @@
PARROT_API
PARROT_WARN_UNUSED_RESULT
PARROT_CANNOT_RETURN_NULL
-void* stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p))
+Stack_Entry_t* stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t
**stack_p))
__attribute__nonnull__(1)
__attribute__nonnull__(2)
FUNC_MODIFIES(*stack_p);
diff -r -u parrot-x64-before/src/stack_common.c parrot-x64/src/stack_common.c
--- parrot-x64-before/src/stack_common.c 2008-04-25 07:55:48.000000000
-0400
+++ parrot-x64/src/stack_common.c 2008-04-25 10:12:20.000000000 -0400
@@ -110,7 +110,7 @@
/*
-=item C<void* stack_prepare_push>
+=item C<Stack_Entry_t* stack_prepare_push>
Return a pointer, where new entries go for push.
@@ -121,7 +121,7 @@
PARROT_API
PARROT_WARN_UNUSED_RESULT
PARROT_CANNOT_RETURN_NULL
-void*
+Stack_Entry_t*
stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p))
{
Stack_Chunk_t * const chunk = *stack_p;
@@ -138,7 +138,7 @@
/*
-=item C<void* stack_prepare_pop>
+=item C<Stack_Entry_t* stack_prepare_pop>
Return a pointer, where new entries are popped off.
@@ -149,7 +149,7 @@
PARROT_API
PARROT_WARN_UNUSED_RESULT
PARROT_CANNOT_RETURN_NULL
-void*
+Stack_Entry_t*
stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p))
{
Stack_Chunk_t * const chunk = *stack_p;
diff -r -u parrot-x64-before/src/stacks.c parrot-x64/src/stacks.c
--- parrot-x64-before/src/stacks.c 2008-04-25 07:55:49.000000000 -0400
+++ parrot-x64/src/stacks.c 2008-04-25 10:14:06.000000000 -0400
@@ -60,7 +60,6 @@
mark_stack(PARROT_INTERP, ARGMOD(Stack_Chunk_t *chunk))
{
for (; ; chunk = chunk->prev) {
- void **entry_data;
Stack_Entry_t *entry;
pobject_lives(interp, (PObj *)chunk);
@@ -68,8 +67,7 @@
if (chunk == chunk->prev)
break;
- entry_data = STACK_DATAP(chunk);
- entry = (Stack_Entry_t *)entry_data;
+ entry = STACK_DATAP(chunk);
if (entry->entry_type == STACK_ENTRY_PMC && UVal_pmc(entry->entry))
pobject_lives(interp, (PObj *)UVal_pmc(entry->entry));
@@ -140,7 +138,6 @@
stack_entry(PARROT_INTERP, ARGIN(Stack_Chunk_t *stack), INTVAL depth)
{
Stack_Chunk_t *chunk;
- void **entry;
size_t offset = (size_t)depth;
if (depth < 0)
@@ -159,9 +156,7 @@
if (chunk == chunk->prev)
return NULL;
- /* this looks bad, but it avoids a type-punning warning */
- entry = STACK_DATAP(chunk);
- return (Stack_Entry_t *)entry;
+ return STACK_DATAP(chunk);
}
/*
--
Andy Dougherty [EMAIL PROTECTED]
