{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/node/issues/19196","repository_url":"https://api.github.com/repos/nodejs/node","labels_url":"https://api.github.com/repos/nodejs/node/issues/19196/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/node/issues/19196/comments","events_url":"https://api.github.com/repos/nodejs/node/issues/19196/events","html_url":"https://github.com/nodejs/node/pull/19196","id":303035442,"number":19196,"title":"node_crypto: Use math.h definitions of isnan and isinf","user":{"login":"jer-gentoo","id":2178449,"avatar_url":"https://avatars0.githubusercontent.com/u/2178449?v=4","gravatar_id":"","url":"https://api.github.com/users/jer-gentoo","html_url":"https://github.com/jer-gentoo","followers_url":"https://api.github.com/users/jer-gentoo/followers","following_url":"https://api.github.com/users/jer-gentoo/following{/other_user}","gists_url":"https://api.github.com/users/jer-gentoo/gists{/gist_id}","starred_url":"https://api.github.com/users/jer-gentoo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jer-gentoo/subscriptions","organizations_url":"https://api.github.com/users/jer-gentoo/orgs","repos_url":"https://api.github.com/users/jer-gentoo/repos","events_url":"https://api.github.com/users/jer-gentoo/events{/privacy}","received_events_url":"https://api.github.com/users/jer-gentoo/received_events","type":"User","site_admin":false},"labels":[{"id":155267502,"url":"https://api.github.com/repos/nodejs/node/labels/C++","name":"C++","color":"e11d21","default":false},{"id":155267357,"url":"https://api.github.com/repos/nodejs/node/labels/crypto","name":"crypto","color":"009800","default":false},{"id":582285981,"url":"https://api.github.com/repos/nodejs/node/labels/v8.x","name":"v8.x","color":"1d76db","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":23,"created_at":"2018-03-07T10:04:56Z","updated_at":"2018-03-27T17:51:34Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/nodejs/node/pulls/19196","html_url":"https://github.com/nodejs/node/pull/19196","diff_url":"https://github.com/nodejs/node/pull/19196.diff","patch_url":"https://github.com/nodejs/node/pull/19196.patch"},"body":"Unless you specify C++11, std::isnan and std::isinf are not guaranteed to be available. Instead, just use the math.h functions for now.\r\n\r\n<!--\r\nThank you for your pull request. Please provide a description above and review\r\nthe requirements below.\r\n\r\nBug fixes and new features should include tests and possibly benchmarks.\r\n\r\nContributors guide: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md\r\n-->\r\n\r\n##### Checklist\r\n<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->\r\n\r\n- [ ] `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes\r\n- [ ] tests and/or benchmarks are included\r\n- [ ] documentation is changed or added\r\n- [ ] commit message follows [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)\r\n"},"comment":{"url":"https://api.github.com/repos/nodejs/node/issues/comments/376615794","html_url":"https://github.com/nodejs/node/pull/19196#issuecomment-376615794","issue_url":"https://api.github.com/repos/nodejs/node/issues/19196","id":376615794,"user":{"login":"yhwang","id":1147320,"avatar_url":"https://avatars2.githubusercontent.com/u/1147320?v=4","gravatar_id":"","url":"https://api.github.com/users/yhwang","html_url":"https://github.com/yhwang","followers_url":"https://api.github.com/users/yhwang/followers","following_url":"https://api.github.com/users/yhwang/following{/other_user}","gists_url":"https://api.github.com/users/yhwang/gists{/gist_id}","starred_url":"https://api.github.com/users/yhwang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yhwang/subscriptions","organizations_url":"https://api.github.com/users/yhwang/orgs","repos_url":"https://api.github.com/users/yhwang/repos","events_url":"https://api.github.com/users/yhwang/events{/privacy}","received_events_url":"https://api.github.com/users/yhwang/received_events","type":"User","site_admin":false},"created_at":"2018-03-27T17:51:34Z","updated_at":"2018-03-27T17:51:34Z","author_association":"MEMBER","body":"Let me post my findings here:\r\n```\r\ng++ '-DV8_GYP_BUILD' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' -I../deps/v8 -I../. -I../deps/v8/include  -pthrea\r\nd -Wall -Wextra -Wno-unused-parameter -m64 -fno-strict-aliasing -m64 -fdata-sections -ffunction-sections -O3 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=c++11 -MMD -MF /home/vagrant/WS_git/node/ou\r\nt/Release/.deps//home/vagrant/WS_git/node/out/Release/obj.target/v8_libplatform/deps/v8/src/libplatform/tracing/trace-config.o.d.raw   -c -o /home/vagrant/WS_git/node/out/Release/obj.target/v8_libplatform/deps/v\r\n8/src/libplatform/tracing/trace-config.o ../deps/v8/src/libplatform/tracing/trace-config.cc\r\nIn file included from ../deps/v8/src/unicode.h:10:0,\r\n                 from ../deps/v8/src/runtime/runtime.h:14,\r\n                 from ../deps/v8/src/interpreter/interpreter.h:17,\r\n                 from ../deps/v8/src/setup-isolate-full.cc:8:\r\n../deps/v8/src/utils.h: In function 'T v8::internal::JSMax(T, T)':\r\n../deps/v8/src/utils.h:171:7: error: '__builtin_isnan' is not a member of 'std'\r\n   if (std::isnan(x)) return x;\r\n```\r\n\r\nThe GCC version I used:\r\n```\r\nvagrant@vagrant:~/WS_git/node$ gcc -v\r\nUsing built-in specs.\r\nCOLLECT_GCC=gcc\r\nCOLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper\r\nTarget: x86_64-linux-gnu\r\nConfigured with: ../src/configure -v --with-pkgversion='Ubuntu 5.5.0-1ubuntu2' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu\r\nThread model: posix\r\ngcc version 5.4.1 20171010 (Ubuntu 5.5.0-1ubuntu2)\r\n```\r\nI used `v8.x` branch:\r\n```\r\nvagrant@vagrant:~/WS_git/node$ git status\r\nOn branch v8.x\r\nYour branch is up-to-date with 'origin/v8.x'.\r\n\r\nChanges not staged for commit:\r\n  (use \"git add <file>...\" to update what will be committed)\r\n  (use \"git checkout -- <file>...\" to discard changes in working directory)\r\n\r\n        modified:   common.gypi\r\n\r\nno changes added to commit (use \"git add\" and/or \"git commit -a\")\r\nvagrant@vagrant:~/WS_git/node$ git remote -v\r\norigin  https://github.com/nodejs/node.git (fetch)\r\norigin  https://github.com/nodejs/node.git (push)\r\n\r\nvagrant@vagrant:~/WS_git/node$ git diff\r\ndiff --git a/common.gypi b/common.gypi\r\nindex 99351610eb..c6d64161fd 100644\r\n--- a/common.gypi\r\n+++ b/common.gypi\r\n@@ -287,7 +287,7 @@\r\n       }],\r\n       [ 'OS in \"linux freebsd openbsd solaris android aix\"', {\r\n         'cflags': [ '-Wall', '-Wextra', '-Wno-unused-parameter', ],\r\n-        'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++0x' ],\r\n+        'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=c++11' ],\r\n         'ldflags': [ '-rdynamic' ],\r\n         'target_conditions': [\r\n           # The 1990s toolchain on SmartOS can't handle thin archives.\r\n```\r\nAs you can see, even I added `std=c++11`, I still failed to compile the code. I tried to use `gnu++11` and still got the same compile errors.\r\n\r\n@jer-gentoo The situation is that when using `cmath`, you should use `std::isnan`. And in our case, we do use `cmath` indirectly, because of `algorithm` header. and like the reference your point to (http://en.cppreference.com/w/cpp/numeric/math/isnan), it uses `std::isnan` in the example code.\r\n\r\nWe all want to resolve the issue: the `v8.x` branch can't compile in some specific GCC versions and not only `node_crypto.cc` but also some code in `deps/v8`. I also found an old issue which should be related to this: https://github.com/nodejs/node-v0.x-archive/issues/9351 . I will try `_GLIBCXX_USE_C99_MATH` and update the results later."}}
{"actor":{"display_login":"yhwang"}}