phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.83k stars 199 forks source link

Compilation error: "no matching function for call to 'v8::ObjectTemplate::SetAccessor" #528

Open viktym opened 5 months ago

viktym commented 5 months ago

During the compilation got an error error: no matching function for call to 'v8::ObjectTemplate::SetAccessor(v8::Local<v8::String>, void (&)(v8::Local<v8::String>, const v8::PropertyCallbackInfo<v8::Value>&), NULL, v8::Local<v8::External>, v8::AccessControl, v8::PropertyAttribute)' 83 | php_obj->SetAccessor(V8JS_STRL(ZSTR_VAL(property_name), static_cast<int>(ZSTR_LEN(property_name))), v8js_fetch_php_variable, NULL, v8::External::New(isolate, ctx), v8::DEFAULT, v8::ReadOnly); image

Does anyone face it? Any recommendations?

alexwight commented 5 months ago

I've faced this problem today. Got it compiling however not sure if it'll introduce issues when I start using it.

I dropped the last 2 parameters to the SetAccessor call in v8js_variables.cc

php_obj->SetAccessor(V8JS_STRL(ZSTR_VAL(property_name), static_cast<int>(ZSTR_LEN(property_name))),v8js_fetch_php_variable,NULL,v8::External::New(isolate, ctx));

The extension builds and the tests mostly now pass - with only 2 failures

make test

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test V8::executeString() : Issue #306 V8 crashing on toLocaleString() [tests/issue_306_basic.phpt]
Test V8::executeString() : Check timezone handling [tests/timezones.phpt]
=====================================================================

Given they are both related to Date/Time handling I'm currently guessing it's unrelated so just going to steam forward for now.

Hope this helps

redbullmarky commented 4 months ago

The recent V8Js was made in response to API changes in more recent versions of V8. It actually looks like they've gone further too and SetAccessor is on its way out:

https://github.com/v8/v8/commit/e48c47268c53

@alexwight / @viktym what version of V8 are you compiling against? or can you give SetAccessorProperty a go in place of SetAccessor and see if that works?

looksystems commented 4 months ago

I have the same problem on OSX with php 8.3.3:

brew install v8 (which fetches version 12.1.285.24) then following the instructions on the README.MacOS.md

gives the following error:

/tmp/v8js/v8js_variables.cc:83:12: **error: no matching member function for call to 'SetAccessor'**
                php_obj->SetAccessor(V8JS_STRL(ZSTR_VAL(property_name), static_cast<int>(ZSTR_LEN(property_name))), v8js_fetch_php_variable, NULL, v8::External::New(isolate, ctx), v8::DEFAULT, v8::ReadOnly);

As per @alexwight, i tried to drop the last to params which compiles but, unfortunately, make test fails 98% of tests.

Any tips?

@redbullmarky - How might I "give SetAccessorProperty a go" ?

Thank you.

alexwight commented 4 months ago

I was building using php 8.2.17 and v8 12.1.285.24 but just switched to php 8.3.7 and I'm getting the same result as before:

=====================================================================
PHP         : /opt/homebrew/Cellar/php/8.3.7/bin/php
PHP_SAPI    : cli
PHP_VERSION : 8.3.7
ZEND_VERSION: 4.3.7
PHP_OS      : Darwin - Darwin alex-iMac 23.4.0 Darwin Kernel Version 23.4.0: Wed Feb 21 21:44:06 PST 2024; root:xnu-10063.101.15~2/RELEASE_ARM64_T8103 arm64
INI actual  : /Users/alexwight/projects/v8/v8js/tmp-php.ini
More .INIs  :
---------------------------------------------------------------------

...

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :     0
Exts tested     :    63
---------------------------------------------------------------------

Number of tests :   181               180
Tests skipped   :     1 (  0.6%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :     2 (  1.1%) (  1.1%)
Tests passed    :   178 ( 98.3%) ( 98.9%)
---------------------------------------------------------------------
Time taken      :     9 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test V8::executeString() : Issue #306 V8 crashing on toLocaleString() [tests/issue_306_basic.phpt]
Test V8::executeString() : Check timezone handling [tests/timezones.phpt]
=====================================================================

98% of tests are passing.

@redbullmarky i'll try and give SetAccessorProperty a go as soon as I can and report back

looksystems commented 4 months ago

Quick update:

I added -DV8_ENABLE_SANDBOX to the configure command:

./configure --with-v8js=/opt/homebrew CPPFLAGS="-DV8_COMPRESS_POINTERS -DV8_ENABLE_SANDBOX"

and am now getting a 98% pass with the two params removed from SetAccessor function call

alexwight commented 4 months ago

Quick update:

I added -DV8_ENABLE_SANDBOX to the configure command:

./configure --with-v8js=/opt/homebrew CPPFLAGS="-DV8_COMPRESS_POINTERS -DV8_ENABLE_SANDBOX"

and am now getting a 98% pass with the two params removed from SetAccessor function call

Sorry yes, I had that too - I should have mentioned that 😅