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

[mono] Assume C99 support #51504

Merged
merged 1 commit into from
May 4, 2021
Merged

[mono] Assume C99 support #51504

merged 1 commit into from
May 4, 2021

Conversation

CoffeeFlux
Copy link
Contributor

We don't seem to have been setting this define properly, but we can just remove the ifdef entirely

@ghost
Copy link

ghost commented Apr 19, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

We don't seem to have been setting this define properly, but we can just remove the ifdef entirely

Author: CoffeeFlux
Assignees: -
Labels:

area-VM-meta-mono

Milestone: -

@lambdageek
Copy link
Member

lambdageek commented Apr 20, 2021

We might be better off using the #else definitions of all that stuff. The C99 standard variadic macros are a mess https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html that don't expand how you want anyway.

I think we could put a G_ATTR_FORMAT_PRINTF on g_log and get the benefit of warnings if the format string is incorrectly specified).

@vargaz
Copy link
Contributor

vargaz commented Apr 20, 2021

Older msvc versions didn't have c99 support, newer ones might have it.

@tannergooding
Copy link
Member

newer ones might have it.

AFAIK it still doesn't and things like _Complex and variable length arrays are still not supported.

@CoffeeFlux
Copy link
Contributor Author

I believe they've committed to supporting all required C11 features, but that does mean some of the C99 stuff like VLAs isn't supported (since they were made optional).

Also seeing the build failures, I'm almost certainly just going to switch to the #else definitions here. I just haven't had a chance to update the PR.

We don't seem to have been setting this define properly, but we can just remove the ifdef entirely
@CoffeeFlux CoffeeFlux merged commit 788a85e into dotnet:main May 4, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants