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.

Sign up for GitHub

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

Jump to bottom

Support compiling on msys2/ucrt64 (no exception handling) #254

Merged
merged 1 commit into from
Dec 31, 2023
Merged

Support compiling on msys2/ucrt64 (no exception handling) #254

merged 1 commit into from
Dec 31, 2023

Conversation

qmfrederik
Copy link
Collaborator

This contains a minimal set of changes from #190 to get libobjc2 to compile under msys2/ucrt64. Exception handling is broken, but it at least allows for some kind of testing with libobjc2 on that environment.

I had to configure the project with CC=clang CXX=clang LDFLAGS="-fuse-ld=lld -lstdc++ -lgcc_s" cmake .. -DTESTS=ON.
79% tests passed, 40 tests failed out of 194.

I hope these changes are non-controversial and getting them merged into master prevent them from bit rotting on a branch (like #190) - further work could come as smaller, follow-up PRs.

Any objections?

@qmfrederik qmfrederik force-pushed the 2.1-mingw branch 2 times, most recently from a0f685d to 28836e4 Compare December 29, 2023 12:21
hmelder
hmelder reviewed Dec 29, 2023
View reviewed changes
objc_msgSend.x86-32.S Outdated
.ascii " /EXPORT:_objc_msgSend"
.ascii " /EXPORT:_objc_msgSend_stret"
.ascii " /EXPORT:_objc_msgSend_fpret"
#else
.ascii " /EXPORT:objc_msgSend"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the CDECL macro defined in common.S to avoid duplication of directives. You might need to change the ifdefs tho

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libobjc2/common.S

Lines 1 to 5 in 8e18060

#if ((defined(_WIN32) || defined(__CYGWIN__)) && defined(__i386__)) || defined(__APPLE__)
#define CDECL(symbol) _##symbol
#else
#define CDECL(symbol) symbol
#endif

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure about using the existing macros, but I added a new macro to common.S which simplified things. Thanks!

davidchisnall
davidchisnall approved these changes Dec 30, 2023
View reviewed changes
objc_msgSend.x86-32.S
@@ -37,7 +37,7 @@
test %eax, %eax
jz 5f # Nil slot - invoke some kind of forwarding mechanism
mov SLOT_OFFSET(%eax), %ecx
#ifdef _WIN32
#ifdef _MSC_VER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does MSYS not support CFG?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this recently landed: https://sourceforge.net/p/mingw-w64/mailman/message/37712869/, I’ll have a closer look later today/tomorrow

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mingw does support CFG, I've reverted this change.

@davidchisnall
Copy link
Member

Looks good to me. Please can you rebase and squash?

@qmfrederik
Copy link
Collaborator Author

Thanks, @davidchisnall . I rebased and squashed the commits. Looks like the FreeBSD build failures on this PR and #255 are unrelated:

pkg install -y cmake ninja git
Updating FreeBSD repository catalogue...
pkg: http://pkgmir.geo.freebsd.org/FreeBSD:12:amd64/quarterly/meta.txz: Not Found
repository FreeBSD has no meta file, using default settings
pkg: http://pkgmir.geo.freebsd.org/FreeBSD:12:amd64/quarterly/packagesite.pkg: Not Found
pkg: http://pkgmir.geo.freebsd.org/FreeBSD:12:amd64/quarterly/packagesite.txz: Not Found
Unable to update repository FreeBSD
Error updating repositories!

@qmfrederik
Copy link
Collaborator Author

Rebased on master

@davidchisnall davidchisnall merged commit a61309b into gnustep:master Dec 31, 2023
41 checks passed
@triplef triplef mentioned this pull request Jan 2, 2024
Closed
@qmfrederik qmfrederik mentioned this pull request Feb 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hmelder hmelder hmelder left review comments

@davidchisnall davidchisnall davidchisnall approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qmfrederik @davidchisnall @hmelder