Page MenuHomePhabricator

Eina value: fix test on Windows
Needs RevisionPublic

Authored by vtorri on Oct 2 2020, 2:07 AM.

Details

Summary

On Windows, long is a 32 bits type, so some tests must not be here

Test Plan

running tests

Diff Detail

Repository
rEFL core/efl
Branch
vtorri_eina_test_fix
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17360
Build 11622: arc lint + arc unit
vtorri created this revision.Oct 2 2020, 2:07 AM
vtorri requested review of this revision.Oct 2 2020, 2:08 AM
raster requested changes to this revision.Oct 5 2020, 3:43 AM

shouldn't this then just test 32bit values on windows rather than test nothing?

This revision now requires changes to proceed.Oct 5 2020, 3:43 AM
vtorri added a comment.Oct 5 2020, 3:54 AM

you can't have a cross-compiled way to use eina_value with long . So i think it would be better to deprecated eina_value with long (imho)

and for 32 bits, int is already tested

vtorri added a comment.Oct 5 2020, 3:55 AM

doing a test with long and 32 bits value just hide the problem

jptiz added a comment.Oct 29 2020, 1:26 PM

you can't have a cross-compiled way to use eina_value with long . So i think it would be better to deprecated eina_value with long (imho)

and for 32 bits, int is already tested

I'm going to give my two cents to this discussion.

So, seeing the EINA_VALUE_TYPE_LONG Documentation, it doesn't specify any data type size, so I think it relies on machine/compiler internal implementation, which is ok for C and C++, but starts raising issues when related to bindings (e.g. C# long). Not big issues, but they wouldn't exist if, instead of having long as a possibility, we just rely on the user to decide if he/she wants a signed or unsigned 32 or 64 bit integer using INT, UINT, INT64 and UINT64, which is way more accurate to his/her intention in having a specific sized integer. After all, we already don't have EINA_VALUE_TYPE_LONG_LONG, and instead we have EINA_VALUE_TYPE_INT64.

I you agree, I think that, as @vtorri stated, deprecating eina_value_type_long should be the way.

walac added inline comments.Nov 3 2020, 4:28 AM
src/tests/eina/eina_test_value.c
105

Wouldn't be better to test the size of long, like if (sizeof(long) >= 8) ?