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

fix C4576 compilation error when compiling CRoaring in C++ on Windows #600

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

SalvatorePreviti
Copy link
Contributor

Compiling CRoaring on Windows (tried on Windows 2019 in CI for roaring-node) causes a compilation error due to the use of (cast){initializer} used in C roaring source code.

(roaring_container_iterator_t){ ... } does not compile in Visual Studio when compiling in C++,
it fails with C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax

For reference, https://github.com/SalvatorePreviti/roaring-node/actions/runs/8102601181/job/22145475666

I think the solution is to replace (roaring_container_iterator_t){ ... } with roaring_container_iterator_t{ ... } depending if compiling for C or C++.

This PR defines a macro in containers.c that is (roaring_container_iterator_t) when compiling in C and roaring_container_iterator_t when compiling in C++

issue #599

Salvatore Previti added 2 commits February 29, 2024 23:38
@SalvatorePreviti SalvatorePreviti changed the title fix C4576 fix C4576 compilation error when compiling CRoaring in C++ on Windows Feb 29, 2024
@lemire
Copy link
Member

lemire commented Mar 1, 2024

@SalvatorePreviti We are getting errors with the Windows runners. I'll relaunch them in case it is a fluke.

@lemire
Copy link
Member

lemire commented Mar 1, 2024

Nope. Looks like the tests are failing under Windows.

@SalvatorePreviti
Copy link
Contributor Author

Ok I will look into it thank you

@lemire
Copy link
Member

lemire commented Mar 1, 2024

The problem may not be with this PR. I am investigating.

@lemire
Copy link
Member

lemire commented Mar 1, 2024

The problem wasn't there 2 weeks ago, but it may not be caused by your PR:

Screenshot 2024-03-01 at 10 08 24 AM

@lemire
Copy link
Member

lemire commented Mar 1, 2024

Oh. Right. Now I remember.

@lemire
Copy link
Member

lemire commented Mar 1, 2024

It is an issue identified by @Dr-Emann

We think it is a visual studio bug.

@lemire
Copy link
Member

lemire commented Mar 1, 2024

I am going to merge your PR because I do not think that the failure is related.

@lemire lemire merged commit 290e204 into master Mar 1, 2024
34 of 36 checks passed
@lemire
Copy link
Member

lemire commented Mar 1, 2024

See #603

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

Successfully merging this pull request may close these issues.

2 participants