-
Notifications
You must be signed in to change notification settings - Fork 988
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 foreign test for i3nt #860
Conversation
Do you want to update mats/ftype.h too? |
That seems like a good idea. It turns out that |
For some build environments, `_WIN32` is needed in "foreign3.c" (as in other "foreign*.c" files) instead of `WIN32` without the leading underscore. Also, simplify "mats/build.zuo" by using `_WIN32` in "ftype.h".
4e972ce
to
e363465
Compare
There are many other files in Chez Scheme that use WIN32 without the underscore:
|
Would it make more sense to convert the few _WIN32 cases to use WIN32, and then make sure WIN32 is defined in all the build environments?
|
Microsoft documents _WIN32, so maybe it would make more sense to convert all the WIN32 instances to _WIN32? https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170 |
The build issues for files that make up the kernel are a little different than for files for tests (e.g., tests don't need to cross-compile, and they don't need the various linking modes that the kernel might be compiled for on Windows). I'm inclined to leave things alone in the "c" directory, because C-level build changes so often have surprises. That said, I do think it would make sense switch to |
Let's fix the ones in mats to use _WIN32 and leave WIN32 for the rest. That shouldn't rock the boat. The ones in csug work because the example compiler command uses -DWIN32. |
Sounds great to me - thanks! |
For some build environments,
_WIN32
is needed (as in other "foreign*.c" files) instead ofWIN32
without the leading underscore.