Skip to content

fix build on PHP 8.6+#97

Merged
kjdev merged 1 commit into
kjdev:masterfrom
pavetheway91:fix-php86
Jun 3, 2026
Merged

fix build on PHP 8.6+#97
kjdev merged 1 commit into
kjdev:masterfrom
pavetheway91:fix-php86

Conversation

@pavetheway91

@pavetheway91 pavetheway91 commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

reference:
php/php-src@7114314

Summary by CodeRabbit

  • Chores
    • Standardized internal offset handling across compression contexts to improve consistency and maintainability, reducing risk of subtle initialization issues.

Note: This update contains no user-visible changes or new features.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ec965f42-ff5f-4748-a54b-743b45b50a6b

📥 Commits

Reviewing files that changed from the base of the PR and between 471e7fb and 698b938.

📒 Files selected for processing (1)
  • zstd.c
🚧 Files skipped from review as they are similar to previous changes (1)
  • zstd.c

📝 Walkthrough

Walkthrough

This PR standardizes pointer offset computation in the PHP Zstd extension by replacing XtOffsetOf with the standard C offsetof macro in three locations within zstd.c.

Changes

Standardize object offset computation

Layer / File(s) Summary
Replace XtOffsetOf with offsetof across object context code
zstd.c
Replace XtOffsetOf(php_zstd_context, std) with offsetof(php_zstd_context, std) in php_zstd_context_from_obj() (line 102) and in both compress and uncompress context object handler initialization (lines 1776, 1804).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

I nibble bytes and hop between,
Replacing quirks with C routine,
XtOffsetOf slips out of sight,
offsetof now does it right,
A tidy patch — carrot-sweet delight! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix build on PHP 8.6+' accurately describes the main change: replacing XtOffsetOf with offsetof macros to fix build compatibility for PHP 8.6+.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
zstd.c

zstd.c:28:10: fatal error: 'php.h' file not found
28 | #include <php.h>
| ^~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/698b938026f78d69aa754e1b5dd21300d6efaa12-2f57b45144c7d836/tmp/clang_command_.tmp.9fdd2d.txt
++Contents of '/tmp/coderabbit-infer/698b938026f78d69aa754e1b5dd21300d6efaa12-2f57b45144c7d836/tmp/clang_command_.tmp.9fdd2d.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-triple"
"x86_64-unknown-linux-gnu" "-emit-obj" "-mrelax-all" "-disable-free"
"-clear-ast-before-backend" "-disable

... [truncated 635 characters] ...

ystem"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/lib/clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-ferror-limit" "19" "-fgnuc-version=4.2.1" "-fskip-odr-check-in-gmf"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/2f57b45144c7d836/file.o" "-x" "c" "zstd.c" "-O0"
"-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pavetheway91

Copy link
Copy Markdown
Contributor Author

closed this accidentally by messing with the wrong branch

@pavetheway91 pavetheway91 reopened this Jun 2, 2026
@kjdev
kjdev merged commit 9705ad1 into kjdev:master Jun 3, 2026
@kjdev

kjdev commented Jun 3, 2026

Copy link
Copy Markdown
Owner

Thanks.

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