Skip to content

Conversation

@akosthekiss
Copy link
Member

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu

@zherczeg
Copy link
Member

I would like to see the effect on some exotic compiler environments. This has never caused any issues on Linux.

@akosthekiss
Copy link
Member Author

@zherczeg Please, name those exotic compiler environments and also those from whom you expect the feedback. Right now, I see no setup where ? true : false would be a sensible construct.

@LaszloLango
Copy link
Contributor

LaszloLango commented Oct 29, 2018

LGTM if builds in the same environment @rtakacs uses. @rtakacs please validate the patch.

@rtakacs
Copy link
Contributor

rtakacs commented Oct 29, 2018

I've tried out JSRemoteTest with this branch on TizenRT target (compiler: arm-none-eabi-gcc - 4.9.3, TizenRT hash: 766372e):

$ python -m jstest --device artik053 --device-id /dev/ARTIK053 --app jerryscript

cd /home/rtakacs/Projects/js-remote-test/deps/jerryscript/build/jerry-core && /usr/bin/arm-none-eabi-gcc  -DCONFIG_MEM_HEAP_AREA_SIZE=71680 -DJERRY_NDEBUG -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/include -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/api -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/debugger -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/base -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/builtin-objects -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/builtin-objects/typedarray -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/operations -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/jcontext -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/jmem -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/jrt -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/lit -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/js -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/regexp -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/vm -I/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-libm/include  -mcpu=cortex-r4 -mfpu=vfpv3 -fno-builtin -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -Wall -Werror -Wshadow -Wno-error=conversion -std=c99 -pedantic -fno-builtin -fno-stack-protector -Wall -Werror=all -Wextra -Werror=extra -Wformat-nonliteral -Werror=format-nonliteral -Winit-self -Werror=init-self -Wconversion -Werror=conversion -Wsign-conversion -Werror=sign-conversion -Wformat-security -Werror=format-security -Wmissing-declarations -Werror=missing-declarations -Wshadow -Werror=shadow -Wstrict-prototypes -Werror=strict-prototypes -Wundef -Werror=undef -Wold-style-definition -Werror=old-style-definition -Wno-stack-protector -Wno-attributes -Werror -Wlogical-op -Werror=logical-op -isystem /home/rtakacs/Projects/js-remote-test/deps/tizenrt/os/include -Os -DNDEBUG   -o CMakeFiles/jerry-core.dir/parser/js/js-parser-util.c.obj   -c /home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/js/js-parser-util.c
In file included from /home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/base/ecma-alloc.h:19:0,
                 from /home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/js/js-lexer.c:16:
/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/js/js-lexer.c: In function 'lexer_construct_regexp_object':
/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/ecma/base/ecma-globals.h:246:3: error: conversion to '_Bool8' from 'long int' may alter its value [-Werror=conversion]
   (JERRY_UNLIKELY ((value) == ECMA_VALUE_ERROR))
   ^
/home/rtakacs/Projects/js-remote-test/deps/jerryscript/jerry-core/parser/js/js-lexer.c:2178:19: note: in expansion of macro 'ECMA_IS_VALUE_ERROR'
   bool is_throw = ECMA_IS_VALUE_ERROR (completion_value);

@rtakacs
Copy link
Contributor

rtakacs commented Oct 29, 2018

More detailed output: https://gist.github.com/rtakacs/0de9c0fa1b9a1b4b80162e435dd8f947

An other info: JSRemoteTest uses newer version of TizenRT (Jun 5, 2018) than JerryScript's Travis (Oct 20, 2017).

@akosthekiss akosthekiss force-pushed the no-ternary-true-false branch from 2ae2e8a to 6c5d2aa Compare October 29, 2018 10:45
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
@akosthekiss akosthekiss force-pushed the no-ternary-true-false branch from 6c5d2aa to 2f85e8f Compare October 29, 2018 10:46
@akosthekiss
Copy link
Member Author

@rtakacs I've updated the PR. I did not expect JERRY_UNLIKELY (the expression behind ECMA_IS_VALUE_ERROR) to yield result incompatible with bool. Now, that result is also cast explicitly. Could you please run a new round of tests?

@rtakacs
Copy link
Contributor

rtakacs commented Oct 29, 2018

@akosthekiss Okay, it works now :)

LGTM (informal)

@akosthekiss
Copy link
Member Author

Thanks, @rtakacs

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akosthekiss akosthekiss merged commit aeddb1c into jerryscript-project:master Oct 29, 2018
@akosthekiss akosthekiss deleted the no-ternary-true-false branch October 29, 2018 13:52
rtakacs pushed a commit to rtakacs/jerryscript that referenced this pull request Nov 27, 2018
The explicit boolean type conversion (introduced by jerryscript-project#2575) desn't work
with TizenRT if custom boolean reprezentation is used. Of course,
TizenRT gives an opportunity to use the C99 standard boolean (that works
well if that is set).

I've replaced all the explicit boolean type conversions with double
negotions that helps to work JerryScript well with custom boolean types.

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
rtakacs pushed a commit to rtakacs/jerryscript that referenced this pull request Nov 27, 2018
The explicit boolean type conversion (introduced by jerryscript-project#2575) desn't work
with TizenRT if custom boolean representation is used. Of course,
TizenRT gives an opportunity to use the C99 standard boolean (that works
well if that is set).

I've replaced all the explicit boolean type conversions with double
negations that helps to work JerryScript well with custom boolean types.

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
rtakacs pushed a commit to rtakacs/jerryscript that referenced this pull request Nov 27, 2018
The explicit boolean type conversion (introduced by jerryscript-project#2575) desn't work
with TizenRT if custom boolean representation is used. Of course,
TizenRT gives an opportunity to use the C99 standard boolean (that works
well if that is set).

I've replaced all the explicit boolean type conversions with double
negations that helps to work JerryScript well with custom boolean types.

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
rtakacs pushed a commit to rtakacs/jerryscript that referenced this pull request Nov 28, 2018
The explicit boolean type conversion (introduced by jerryscript-project#2575) desn't work
with TizenRT if custom boolean representation is used. Of course,
TizenRT gives an opportunity to use the C99 standard boolean (that works
well if that is set).

I've replaced all the explicit boolean type conversions with double
negations that helps to work JerryScript well with custom boolean types.

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
zherczeg pushed a commit that referenced this pull request Nov 28, 2018
The explicit boolean type conversion (introduced by #2575) desn't work
with TizenRT if custom boolean representation is used. Of course,
TizenRT gives an opportunity to use the C99 standard boolean (that works
well if that is set).

I've replaced all the explicit boolean type conversions with double
negations that helps to work JerryScript well with custom boolean types.

JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants