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]