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

ExternalAsciiStringResource is gone in upstream V8 #222

Closed
bnoordhuis opened this issue Dec 14, 2014 · 6 comments
Closed

ExternalAsciiStringResource is gone in upstream V8 #222

bnoordhuis opened this issue Dec 14, 2014 · 6 comments
Labels

Comments

@bnoordhuis
Copy link
Member

Replaced in v8/v8@299ed09 with 'ExternalOneByteStringResource`. Not sure about the exact version because the V8 people don't tag properly but it was probably a 3.28 release.

This affects iojs because it ships V8 3.30. I'll try to come up with a patch.

@bnoordhuis bnoordhuis added the bug label Dec 14, 2014
@kkoopa
Copy link
Collaborator

kkoopa commented Dec 14, 2014

I'll guess it always was an 8-bit char array, that is, not proper ASCII. If so, a straight replacement should work 1:1, without any filtering or masking out possible high bits. Just need to condition on the right versions somehow. Has io.js got any preprocessor symbol indicating that it is iojs and not joyent node?

@bnoordhuis
Copy link
Member Author

Not at the moment. I'm leaning towards removing the NanNew() overloads for ExternalAsciiStringResource because those won't work across versions anyway. The only other place it's used is in _NanGetExternalParts() and that can probably be made to work through SFINAE.

@kkoopa
Copy link
Collaborator

kkoopa commented Dec 14, 2014

As the type has been removed, it seems like the only way, although I do not like it. It means the 1.5 branch will have to become 2.0, as functionality is removed. The good part there is that All ugly deprecated stuff can be removed, including _NanGetExternalParts, which is only used in the deprecated string converter. Then the changes to NanWeakCallback from #218 have to go in, among with removing the NAN_WEAK_CALLBACK macro.

@bnoordhuis
Copy link
Member Author

@kkoopa I cobbled something together that tries hard not to break the API, please see #223. It probably still warrants a minor bump because I won't vouch all compilers will always infer the type of T in all situations. g++ demonstrably does not so msvc probably does even worse.

@rlidwka
Copy link

rlidwka commented Jan 10, 2015

Any progress on resolving this?

I'm trying to port my binary packages to work under io.js when it's released, and this issue is a blocker. :(

@bnoordhuis
Copy link
Member Author

@rlidwka I updated #223, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants