Skip to content
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

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

Closed
viktym opened this issue Apr 1, 2024 · 6 comments

Comments

@viktym
Copy link

viktym commented Apr 1, 2024

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
Copy link

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

jlyon added a commit to jlyon/v8js that referenced this issue Apr 15, 2024
@redbullmarky
Copy link
Collaborator

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:

v8/v8@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
Copy link

looksystems commented May 15, 2024

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
Copy link

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
Copy link

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
Copy link

alexwight commented May 15, 2024

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 😅

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

No branches or pull requests

4 participants