Warning fixes. #885
Warning fixes. #885
Conversation
| @@ -75,7 +75,8 @@ vm_stack_context_abort (vm_frame_ctx_t *frame_ctx_p, /**< frame context */ | |||
| ecma_collection_chunk_t *chunk_p = MEM_CP_GET_NON_NULL_POINTER (ecma_collection_chunk_t, | |||
| current); | |||
|
|
|||
| ecma_free_value (*(ecma_value_t *) chunk_p->data, true); | |||
| lit_utf8_byte_t *data_ptr = chunk_p->data; | |||
There was a problem hiding this comment.
Is the introduction of this temporary variable really needed? Cannot the expression be kept in one line as before? (2 more similar constructs below.)
There was a problem hiding this comment.
I think this is an alignment problem, perhaps a double casting could help:
ecma_free_value (*(ecma_value_t *) (void *) chunk_p->data, true);
There was a problem hiding this comment.
The chaining type casts doesn't help. The following warning: 'dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]' is still persist in this cases.
robertsipka
force-pushed
the
warning_fixes
branch
from
fba0fb3 to
ce8b5e6
Compare
|
In this case accessing data at 'chunk_p->data' through a pointer to a different type leads the following error: 'Dereferencing type-punned pointer will break strict-aliasing rules'. |
|
@robertsipka, please add |
|
@robertsipka does chaining type casts not help? Something like |
|
Sadly, no. This solution would lead to the same error. |
|
@robertsipka Too bad. But thanks for trying. Ok with me then. |
|
@LaszloLango: Yes, it would be correct way. The remained problem is with the error 'type of bit-field ‘base_cp’ is a GCC extension', as I mentioned in the first comment. |
robertsipka
force-pushed
the
warning_fixes
branch
2 times, most recently
from
d471a74 to
2c376ba
Compare
|
@LaszloLango: I've modified the patch, and rebased with master. |
| mem_cpointer_t getter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to getter object */ | ||
| mem_cpointer_t setter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to setter object */ | ||
| __extension__ mem_cpointer_t getter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to getter object */ | ||
| __extension__ mem_cpointer_t setter_p : ECMA_POINTER_FIELD_WIDTH; /**< pointer to setter object */ |
There was a problem hiding this comment.
Is this working on other compilers not just GCC? On the long run perhaps an unsigned int will also do.
There was a problem hiding this comment.
The following could work somewhere in jrt.h where the attributes are defined.
#ifndef __GNUC__
#define __extension__
#endifBut do note that right now we have no compiler specific ifdefs in the core code base.
There was a problem hiding this comment.
It looks good to me, thanks, I'll define this __ extension __.
|
@robertsipka, thanks. Good to me, but let's wait the review of @zherczeg |
ISO C99 doesn’t support unnamed structs/unions. Comparison of distinct pointer types lacks a cast. Dereferencing type-punned pointer will break strict-aliasing rules. Type of bit-field ‘ext’ is a GCC extension. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka [email protected]
robertsipka
force-pushed
the
warning_fixes
branch
from
2c376ba to
80811c8
Compare
|
Thanks for the comments, I've updated the patch. |
ISO C99 doesn’t support unnamed structs/unions.
Comparison of distinct pointer types lacks a cast.
Dereferencing type-punned pointer will break strict-aliasing rules.
There is still a warning with gcc-4.8.4. (it's supported with gcc 4.9.2.):
type of bit-field ‘ext’ is a GCC extension
In C99, gcc issues a warning with -pedantic but it is permitted to have an implementation defined type for the bit-field.