WordPress / WordPress/wordpress-playground

Stop patching type signatures in PHP source

Open
#652 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

[Feature] PHP.wasm [Priority] Low [Type] Developer Experience
Dominant language
JavaScript
Stars
2k
Forks
462
Avg merge
18h 44m
Merged PRs (30d)
33

Description

Playground applies many PHP patches similar to the one below where php7.1.patch changes zend_ulong to size_t:

-static int pdo_sqlite_stmt_get_col(pdo_stmt_t *stmt, int colno, char **ptr, zend_ulong *len, int *caller_frees)
+static int pdo_sqlite_stmt_get_col(pdo_stmt_t *stmt, int colno, char **ptr, size_t *len, int *caller_frees)

This is needed to silence compiler errors, but I don't have a clear understanding why. GCC is fine with zend_ulong, why isn't Emscripten?

One solution could potentially be defining a macro to remap zend_ulong into size_t, but before that happens it would be great to understand why is the change even needed. Does it have to do with WebAssembly being 32 bit?

The discussion originally started in:

https://github.com/WordPress/wordpress-playground/pull/639#discussion_r1341272020

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the php7.1.patch example and the discussion linked from PR #639, then compare how GCC and Emscripten handle the zend_ulong and size_t signatures. Determine why the patches are required and document or implement a way to remove them, with the affected PHP patches compiling successfully under Emscripten.

Written by the indexing model from the issue text.

Assessment

Tech stack
php, wasm
Domain
build-system, compilers
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.