Skip to content
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

Closed
wants to merge 2 commits into from

Conversation

XadillaX
Copy link

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,
                                 ^

#264

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.

This comment was marked as off-topic.

@kkoopa
Copy link
Collaborator

kkoopa commented Jan 23, 2015

@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 const void * or const char *?

@bnoordhuis
Copy link
Member

@kkoopa That's because of nodejs/node@535fec8 and nodejs/node@56fde66c.

@XadillaX
Copy link
Author

@bnoordhuis So now and later API will stay in const char *?

@bnoordhuis
Copy link
Member

@XadillaX That's correct.

@kkoopa
Copy link
Collaborator

kkoopa commented Jan 23, 2015

Is the bug present in 3.14? Should the UCS2 fix be backported to earlier versions?

@bnoordhuis
Copy link
Member

Is the bug present in 3.14?

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.

@XadillaX
Copy link
Author

AppVeyor build failed.

any suggestion?

@kkoopa
Copy link
Collaborator

kkoopa commented Jan 24, 2015

Ignore it, io.js does not work on windows.

@mathiask88
Copy link
Contributor

Not yet ;) 1.0.4 will address this error I think.

@mscdex
Copy link
Contributor

mscdex commented Jan 24, 2015

+1

@mathiask88
Copy link
Contributor

+1 on getting this fixed, so that my addon becomes io.js ready :)

@XadillaX
Copy link
Author

Will this PR be merged? I'm in a hurry.

@kkoopa
Copy link
Collaborator

kkoopa commented Feb 1, 2015

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

ForbesLindesay added a commit to ForbesLindesay/thread-sleep that referenced this pull request Feb 5, 2015
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
ceejbot pushed a commit to ceejbot/avon that referenced this pull request Feb 5, 2015
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
@hit9
Copy link

hit9 commented Feb 6, 2015

I'm hoping this fixed. :)

@kkoopa
Copy link
Collaborator

kkoopa commented Feb 6, 2015

Fixed through #273.

@kkoopa kkoopa closed this Feb 6, 2015
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.

6 participants