-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix a bug occurred the compilation error #265
Conversation
1.0.3/src/node.h:251:34: note: candidate function not viable: cannot convert argument of incomplete type 'const void *' to 'const char *' NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate, ^
@@ -1847,7 +1847,7 @@ NAN_INLINE v8::Local<v8::Value> NanEncode( | |||
#if (NODE_MODULE_VERSION > 0x000B) | |||
return node::Encode( | |||
v8::Isolate::GetCurrent() | |||
, buf, len | |||
, (const char *)buf, len |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
@bnoordhuis Why was this signature changed in io.js? It also says not to use UCS2. 0.8 only took char * as well, should NanEncode use |
@kkoopa That's because of nodejs/node@535fec8 and nodejs/node@56fde66c. |
@bnoordhuis So now and later API will stay in |
@XadillaX That's correct. |
Is the bug present in 3.14? Should the UCS2 fix be backported to earlier versions? |
In principle, yes, although V8's CopyChars() implementation is just different enough that it doesn't seem to trigger gcc's SSE heuristics. If you want to back-port the change, you have my blessing. |
any suggestion? |
Ignore it, io.js does not work on windows. |
Not yet ;) 1.0.4 will address this error I think. |
+1 |
+1 on getting this fixed, so that my addon becomes io.js ready :) |
Will this PR be merged? I'm in a hurry. |
More or less. I've been holding off because I want to find a way of accounting for the API change regarding UCS2 in io.js. Will however not be backporting changes to older versions, as that was too much work. Been busy with other responsibilities. ETA: the coming week |
Unfortunately version 1.6.2 of nan appears to be broken (nodejs/nan#265) so we pin to 1.5.0 until 1.6.2 comes out
The compiler warnings that remain all represent unfinished work in the module. I need this PR to land before I can update NAN: nodejs/nan#265
I'm hoping this fixed. :) |
Fixed through #273. |
#264