Skip to content

Partially revert PR 42703 to fix embedding with MSVC#44842

Merged
vtjnash merged 1 commit intoJuliaLang:masterfrom
Taaitaaiger:msvc-embedding
Apr 4, 2022
Merged

Partially revert PR 42703 to fix embedding with MSVC#44842
vtjnash merged 1 commit intoJuliaLang:masterfrom
Taaitaaiger:msvc-embedding

Conversation

@Taaitaaiger
Copy link
Copy Markdown
Contributor

@Taaitaaiger Taaitaaiger commented Apr 3, 2022

Due to the changes introduced in PR #42703 embedding Julia with MSVC fails with the current beta. A compile error is triggered in platform.h, and the definition of ios_t is hidden because JL_ATTRIBUTE_ALIGN_PTRSIZE is empty. This PR puts back a bit of the MSVC-related code that was removed to fix those two issues.

…quired to successfully embed Julia with MSVC
@ViralBShah ViralBShah added embedding Embedding Julia using the C API system:windows Affects only Windows labels Apr 3, 2022
@giordano
Copy link
Copy Markdown
Member

giordano commented Apr 3, 2022

Does this supersede #44587?

@Taaitaaiger
Copy link
Copy Markdown
Contributor Author

No, that example doesn't compile without these changes when the current beta is used. I tested it with 1.7, but I found out it didn't work with the beta today.

@barche
Copy link
Copy Markdown
Contributor

barche commented Apr 13, 2022

Can this still be backported to 1.8, or is it too late?

@ViralBShah ViralBShah added the backport 1.8 Change should be backported to release-1.8 label Apr 13, 2022
KristofferC pushed a commit that referenced this pull request Apr 19, 2022
Put back a few lines that were removed by PR $42703 because they are required to successfully embed Julia with MSVC.

(cherry picked from commit 209aad1)
@KristofferC KristofferC mentioned this pull request Apr 19, 2022
67 tasks
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

embedding Embedding Julia using the C API system:windows Affects only Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants