Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

CODING_STANDARDS.md: establish C99 as the implementation language#10631

Merged
Girgias merged 1 commit into
php:masterphp/php-src:masterfrom
CM4all:coding_standards_c99CM4all/php-src:coding_standards_c99Copy head branch name to clipboard
Feb 20, 2023
Merged

CODING_STANDARDS.md: establish C99 as the implementation language#10631
Girgias merged 1 commit into
php:masterphp/php-src:masterfrom
CM4all:coding_standards_c99CM4all/php-src:coding_standards_c99Copy head branch name to clipboard

Conversation

@MaxKellermann

Copy link
Copy Markdown
Contributor

PHP 8 switched to C99, but did not documented that anywhere.

After @derickr rejected a pull request on timelib
(derickr/timelib#141 (comment)) because my suggested change removed compile-time checks for fixed-width integer types, pointing out that they are optional in the C99 standard, @nikic disagreed with using uint_least8_t instead (which is guaranteed to be available), stating that "We already make extensive use of uint8_t, you can assume it exists" (#10621 (review)).

In order to avoid such confusion in the future, let's document this architecture requirement.

PHP 8 switched to C99, but did not documented that anywhere.

After @derickr rejected a pull request on timelib
(derickr/timelib#141 (comment))
because my suggested change removed compile-time checks for
fixed-width integer types, pointing out that they are optional in the
C99 standard, @nikic disagreed with using `uint_least8_t` instead
(which is guaranteed to be available), stating that "We already make
extensive use of uint8_t, you can assume it exists"
(php#10621 (review)).

In order to avoid such confusion in the future, let's document this
architecture requirement.
@nikic

nikic commented Feb 20, 2023

Copy link
Copy Markdown
Member

Just as a side node, timelib is a library independent from php-src, so both can have different requirements regarding the supported C standard and compiler support. (As long as the php-src requirement >= the timelib requirement.)

That said, the only reasonable environment in which the uintN_t types may not be available is an environment where CHAR_BITS is not 8. Clang doesn't even support such targets in the first place :)

@MaxKellermann

Copy link
Copy Markdown
Contributor Author

Of course, we all know the world has settled on machines where all of those are available and nobody bothers to support theoretical machines where it isn't, but that assumption should be documented.

Prior to PHP8, these configure-/compile-time checks existed not to support those theoretical machines, but because C89 did not have any fixed-width integers. The removal of those checks does not break PHP (or timelib) on theoretical machines; it was already broken before, and those checks did not help with that.

@Girgias Girgias merged commit 5bfd3fa into php:master Feb 20, 2023
@MaxKellermann MaxKellermann deleted the coding_standards_c99 branch March 7, 2023 13:25
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.