Fix JerryScript build with clang-6.0 #2610
Conversation
DanielBallaSZTE
force-pushed
the
fix_clang_build
branch
from
ab42e71 to
63a57b1
Compare
DanielBallaSZTE
force-pushed
the
fix_clang_build
branch
from
63a57b1 to
4d2e5b2
Compare
| jerry_value_t jerry_create_regexp (const jerry_char_t *pattern, jerry_regexp_flags_t flags); | ||
| jerry_value_t jerry_create_regexp_sz (const jerry_char_t *pattern, jerry_size_t pattern_size, | ||
| jerry_regexp_flags_t flags); | ||
| jerry_value_t jerry_create_regexp (const jerry_char_t *pattern, uint16_t flags); |
There was a problem hiding this comment.
Is there a reasion why this is limited to 16 bits? Usually this takes a register regardless of type. I would use a 32 bit value.
There was a problem hiding this comment.
We could use a 32 bit value, however that would go way out of the scope of this patch.
In that case we must change everything related to ecma_op_create_regexp_object. I would like to leave it as it is in this patch, and create a follow up, that changes every related function's parameters to 32 bit values.
There was a problem hiding this comment.
I think @DanielBallaSZTE is right here. The API mirrors the internals.
On the other hand, if the goal is to break away from the internals, then I'd suggest not to use a bitwidth-fixed (e.g., 32 bit) type, but a simple (unsigned?) int instead. Actually, that should be the fastest and easiest and most future-proof type on any machine.
There was a problem hiding this comment.
@akosthekiss Using an unsigned int requires you to cast it to unsigned short int (uint16_t) when calling the ecma_op_create_regexp_object. I already have a patch prepared that changes all the internal parameters to uint32_t, so I would like to leave it as an uint16_t for now and submit the patch if this one lands.
There was a problem hiding this comment.
Actually, I was not pushing for a change here. I was only reflecting to @zherczeg 's proposal to change this to uint32_t. And my comment was that if we wanted a change (i.e., to have an API type that was different from the internal type) then we should go for a non-sized type.
| jerry_value_t jerry_create_regexp (const jerry_char_t *pattern, jerry_regexp_flags_t flags); | ||
| jerry_value_t jerry_create_regexp_sz (const jerry_char_t *pattern, jerry_size_t pattern_size, | ||
| jerry_regexp_flags_t flags); | ||
| jerry_value_t jerry_create_regexp (const jerry_char_t *pattern, uint16_t flags); |
There was a problem hiding this comment.
I think @DanielBallaSZTE is right here. The API mirrors the internals.
On the other hand, if the goal is to break away from the internals, then I'd suggest not to use a bitwidth-fixed (e.g., 32 bit) type, but a simple (unsigned?) int instead. Actually, that should be the fastest and easiest and most future-proof type on any machine.
|
@DanielBallaSZTE ping (has this PR been abandoned?) |
|
@akosthekiss Sorry, definitely not, had to work on something else, will update today with the changes, thanks for the ping! |
Clang-6.0 would throw the following error while building
```
jerryscript/jerry-core/api/jerry.c:1527:71: error: implicit conversion loses integer precision:
'jerry_regexp_flags_t' to 'uint16_t' (aka 'unsigned short') [-Werror,-Wconversion]
jerry_value_t ret_val = ecma_op_create_regexp_object (ecma_pattern, flags);
```
This patch fixes the issue.
Also change the `jerry_create_regexp` and `jerry_create_regexp_sz` functions' `flags` parameter to `uint16_t` since the values created with `bitwise inclusive OR` are not part of the enum.
JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]
DanielBallaSZTE
force-pushed
the
fix_clang_build
branch
from
4d2e5b2 to
9b24bde
Compare
Clang-6.0 would throw the following error while building
This patch fixes the issue.
Also change the
jerry_create_regexpandjerry_create_regexp_szfunctions'flagsparameter touint16_tsince the values created withbitwise inclusive ORare not part of the enum.JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]