Skip to content

Remove no longer needed stdint compatibility defines #5886

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

Closed
wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Jul 22, 2020

As we're using C99, we can assume these exist.

@nikic
Copy link
Member Author

nikic commented Jul 22, 2020

@Girgias Somehow I thought you already did this, but apparently not.

@Girgias
Copy link
Member

Girgias commented Jul 22, 2020

@Girgias Somehow I thought you already did this, but apparently not.

From my understanding they might not be actually defined, as they are optional types: https://en.cppreference.com/w/c/types/integer

That's why I didn't do it

@nikic
Copy link
Member Author

nikic commented Jul 22, 2020

@Girgias Right, they are optional if the target does not support the type. However, in this case it's also not possible to define an equivalent type (if it were possible, it would have been defined) and PHP cannot build on such a target.

@Girgias
Copy link
Member

Girgias commented Jul 22, 2020

@Girgias Right, they are optional if the target does not support the type. However, in this case it's also not possible to define an equivalent type (if it were possible, it would have been defined) and PHP cannot build on such a target.

Okay then :D My understanding of the C standard is pretty limited at times, I thought that maybe some weird architecture target which had a 36bit word could compile PHP even if it doesn't have an exact 32bit integer, hence wouldn't declare int32_t.

But I might just be confused, so if all is good should be OK to merge :)

@php-pulls php-pulls closed this in 9359486 Jul 23, 2020
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