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

32-bit build succeeds, throws exceptions at runtime about integer length #9

Closed
bengotow opened this issue Oct 12, 2016 · 2 comments · Fixed by #10
Closed

32-bit build succeeds, throws exceptions at runtime about integer length #9

bengotow opened this issue Oct 12, 2016 · 2 comments · Fixed by #10

Comments

@bengotow
Copy link
Contributor

bengotow commented Oct 12, 2016

Hey! I know these issues are bordering on the obscure, but I'm still fighting with the Windows builds of N1. We target ia32 instead of x64 on Windows because it's more widely supported, and it looks like there are some casting warnings at compile time that result in the module not working when the app runs. I believe this issue is only reproducible if you're targeting a 32-bit architecture.

Here are a few of the warnings:

c:\users\ieuser\documents\n1\node_modules\bettersqlite3\src\objects\int64\../../util/macros.h(33): warning C4146: unary minus operator applied to unsigned type, result still unsigned (..\src\objects\int64\int64.cc) [C:\Users\IEUser\Documents\N1\node_modules\better-sqlite3\build\better_sqlite3.vcxproj]
c:\users\ieuser\documents\n1\node_modules\bettersqlite3\src\objects\int64\../../util/macros.h(33): warning C4146: unary minus operator applied to unsigned type, result still unsigned (..\src\util\data.cc) [C:\Users\IEUser\Documents\N1\node_modules\better-sqlite3\build\better_sqlite3.vcxproj]
c:\users\ieuser\documents\n1\node_modules\better-sqlite3\src\objects\statement\../../util/macros.h(33): warning C4146: unary minus operator applied to unsigned type, result still unsigned (..\src\objects\database\database.cc) [C:\Users\IEUser\Documents\N1\node_modules\better-sqlite3\build\better_sqlite3.vcxproj]

When you run the app, it complains about the 32-bit integers:

932:1011/192644:INFO:CONSOLE(28)] "TypeError: Expected both arguments to be 32 bit signed integers.
    at TypeError (native)
    at Object.<anonymous> (C:\Users\IEUser\Documents\N1\node_modules\better-sqlite3\index.js:8:34)

If you have a Windows machine, you can reproduce this by running this to build. I'm not 100% sure the warnings are causing the issue, but they seem like a very likely culprit. I wonder if there just need to be different types used here: https://github.com/bengotow/better-sqlite3/blame/n1-1.3.6/src/util/macros.h#L32

npm install --msvs_version=2013 --target=1.4.1 --arch=ia32 --target_platform=win32 --dist-url=https://atom.io/download/atom-shell
@bengotow bengotow changed the title 32-bit build not working? 32-bit build succeeds, throws exceptions at runtime about integer length Oct 12, 2016
@JoshuaWise
Copy link
Member

JoshuaWise commented Oct 12, 2016

Unfortunately I don't have a 32-bit machine to test it on, but if you manage to solve the problem, pull requests are very much appreciated.

The first thing I'd try is changing IS_32BIT_INT to this:

inline bool IS_32BIT_INT(double num) {
    return floor(num) == num && num < 2147483648.0 && num >= -2147483648.0;
}

The .0s at the end of the integer literals force them to be doubles

@bengotow
Copy link
Contributor Author

bengotow commented Oct 12, 2016

Thanks! Will try it out—if you want, you can actually run 32-bit NodeJS on any version of Windows. Microsoft did a great job with backward compatibility. (we actually don't bother shipping a 64-bit version of the app.) Will keep you posted.

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

Successfully merging a pull request may close this issue.

2 participants