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

PHP7 create symbol table #1403

Closed
Jurigag opened this issue Mar 17, 2017 · 10 comments
Closed

PHP7 create symbol table #1403

Jurigag opened this issue Mar 17, 2017 · 10 comments
Assignees

Comments

@Jurigag
Copy link
Contributor

Jurigag commented Mar 17, 2017

Is there any reason why create_symbol_table function is not implementend in ZendEngine3? Like all code there is just commented. For example zephir just uses wrong symbol table for example in this issue - phalcon/cphalcon#12648 on php5 just create_symbol_table() before setting variables is enough but what about php7?

@sjinks sjinks self-assigned this Mar 24, 2017
@sjinks
Copy link
Contributor

sjinks commented Mar 27, 2017

As far as I can tell, symbol tables in ZE3 are linked to the execution context, and it is not trivial to replace the existing symbol table with a newly created one.

What I suggest is: because new symbol tables are used mostly to create an isolated scope for require and company, we add a new argument to zephir_require family:

int zephir_require_ret(zval *return_value_ptr, const char *require_path, zval * symbol_table) ZEPHIR_ATTR_NONNULL1(2);

symbol_table is supposed to be an array or NULL; if it is an array, it is used as a symbol table for the require'd file.

The issue is that this would introduce a change to Zephir syntax:

require path, symtable

or, as an alternative, we can create an optimizer for a function (say, require_with_symtable($file, $symtable = NULL)).

What do you guys think?

@sergeyklay
Copy link
Member

I like the second option for now

@Jurigag
Copy link
Contributor Author

Jurigag commented Mar 27, 2017

What does this mean? We would need to create some weird empty array or pass view params with this require?

@sjinks
Copy link
Contributor

sjinks commented Mar 27, 2017

$symtable = ['var1' => 1, 'var2' => true, /* ... */];
require_with_symtable('somefile.php', $symtable);
// somefile.php sees only variables passed in the $symtable as local ones

@Jurigag
Copy link
Contributor Author

Jurigag commented Mar 27, 2017

Oh okay i understand. But then what with calling $this->xyz in such file if it's required in some class? Will this still work?

@sjinks
Copy link
Contributor

sjinks commented Mar 27, 2017

Good question. No idea until I test

@dreamsxin
Copy link
Contributor

Set the scope, like op_array->scope = Z_OBJCE_P(object);

@steffengy
Copy link
Contributor

just FYI:
There is no reason this was not implemented (except being not important enough for the initial port).
And yes it's technically possible (and not really different to how it's done for ZE2).

@sjinks
Copy link
Contributor

sjinks commented Apr 8, 2017

static zend_array* add_symtable(zend_execute_data* ex)
{
    zend_array* old_table;
    zend_array* symbol_table = (zend_array*)emalloc(sizeof(zend_array));
    zend_hash_init(symbol_table, 0, NULL, ZVAL_PTR_DTOR, 0);
    zend_hash_real_init(symbol_table, 0);

    zend_rebuild_symbol_table();
    zend_detach_symbol_table(ex);
    old_table = ex->symbol_table;
    ex->symbol_table = symbol_table;

    return old_table;
}

static void restore_symtable(zend_execute_data* ex, zend_array* tbl)
{
    zend_hash_destroy(ex->symbol_table);
    efree(ex->symbol_table);
    ex->symbol_table = tbl;
    zend_attach_symbol_table(ex);
    zend_rebuild_symbol_table();
}

If add_symtable and del_symtable are implemented as Zend functions, they will need to use ex->prev_execute_data instead of ex.

sjinks added a commit to sjinks/zephir that referenced this issue Apr 9, 2017
@sjinks
Copy link
Contributor

sjinks commented Apr 9, 2017

OK, let us move to #1488 to discuss this further.

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

5 participants