-
Notifications
You must be signed in to change notification settings - Fork 292
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
Add VLA compatibility macro for C89-ish compilers. #452
Conversation
@isotoxin can you review this change? It's one of the changes we need to make for MSVC compatibility. I believe this directly affects your use case. |
Reviewed 1 of 30 files at r1. Comments from Reviewable |
It'll work, but afaik |
2dc9681
to
f0de2a8
Compare
1fff668
to
26c9a35
Compare
Reviewed 25 of 30 files at r1, 7 of 7 files at r2. Comments from Reviewable |
a30d4ee
to
622af37
Compare
Reviewed 26 of 30 files at r1, 5 of 7 files at r2, 3 of 3 files at r3. Comments from Reviewable |
@isotoxin is this something you're interested in reviewing or should we just go ahead and merge it? By the way, yesterday I realised (after someone linked to it) that you've done this already, almost in the same way. Perhaps this change is useful for you to adopt so you have fewer custom changes to maintain. For background: I'm doing this because I want to support Windows properly, and currently our tests run on Wine (on Linux) and always fail. I want to run the tests on Appveyor (a Windows CI platform), and although it's probably possible to use mingw32 on that, I'd like to take this opportunity to improve support for MSVC, probably benefitting the isotoxin client. |
Sorry guys. I dont know why I missed this PR... I was ill, probably so. |
Only one remark: |
This change is