# Mind if I hook the compiler?

- Thread: https://externals.io/message/80720
- Original archive: https://news-web.php.net/php.internals/80720
- Started by Sara Golemon on 2015-01-18T02:58:05+00:00
- Messages: 6
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #80720 — Sara Golemon — 2015-01-18T02:58:05+00:00

URL: https://externals.io/message/80720#80720

https://github.com/php/php-src/pull/1004

Wondering if anyone has an objection to me merging this.  It just
offers extensions a chance to alter the AST prior to bytecode
emission.  Mostly for evil things, but potentially for 3rd-party
optimizers or whatevs.  Does it need an RFC?  The very minor
indirection cost hides itself behind OpCache, so it should harm
anything...

-Sara

---

## #80727 — Joe Watkins — 2015-01-18T07:01:00+00:00

URL: https://externals.io/message/80720#80727 · In reply to: #80720

Morning Sara,

    Great idea, +1, also agree that it doesn't really need an RFC ... but
I'm always wrong ...

Cheers
Joe



> https://github.com/php/php-src/pull/1004
>
> Wondering if anyone has an objection to me merging this.  It just
> offers extensions a chance to alter the AST prior to bytecode
> emission.  Mostly for evil things, but potentially for 3rd-party
> optimizers or whatevs.  Does it need an RFC?  The very minor
> indirection cost hides itself behind OpCache, so it should harm
> anything...
>
> -Sara

---

## #80728 — Pierre Joye — 2015-01-18T07:36:41+00:00

URL: https://externals.io/message/80720#80728 · In reply to: #80720

Hi,


>
> https://github.com/php/php-src/pull/1004
>
> Wondering if anyone has an objection to me merging this.  It just
> offers extensions a chance to alter the AST prior to bytecode
> emission.  Mostly for evil things, but potentially for 3rd-party
> optimizers or whatevs.  Does it need an RFC?  The very minor
> indirection cost hides itself behind OpCache, so it should harm
> anything...

Good idea.

I think it was even planed when AST was introduced. You may check with
Nikita :)

Cheers,
Pierre

---

## #80747 — Nikita Popov — 2015-01-18T16:50:33+00:00

URL: https://externals.io/message/80720#80747 · In reply to: #80720

> https://github.com/php/php-src/pull/1004
>
> Wondering if anyone has an objection to me merging this.  It just
> offers extensions a chance to alter the AST prior to bytecode
> emission.  Mostly for evil things, but potentially for 3rd-party
> optimizers or whatevs.  Does it need an RFC?  The very minor
> indirection cost hides itself behind OpCache, so it should harm
> anything...
>

Totally okay with making that hookable, but not sure if compile_top_stmt is
the best place to do that, at least in the form implemented in the PR.
Assuming that you want to implement a preprocessing pass on the AST the
fast that zend_compile_top_stmt is called recursively might be somewhat
inconvenient. Maybe compile_top_stmt should at least also call
compile_top_stmt instead of zend_compile_top_stmt?

Nikita

---

## #80754 — Sara Golemon — 2015-01-18T17:22:22+00:00

URL: https://externals.io/message/80720#80754 · In reply to: #80747

> Totally okay with making that hookable, but not sure if compile_top_stmt is
> the best place to do that, at least in the form implemented in the PR.
> Assuming that you want to implement a preprocessing pass on the AST the fast
> that zend_compile_top_stmt is called recursively might be somewhat
> inconvenient. Maybe compile_top_stmt should at least also call
> compile_top_stmt instead of zend_compile_top_stmt?
>
I did think about that, and ended up with this spot on the idea that
one could just let the main implementation worry about recursive, and
only alter patters as they're seen.

For example, if my extension wants only wants to do constant folding,
I might wait until I get fed an ast node for looking up constants
rather than drilling down myself, then leaving the engine to drill
down again.

That said, I'm not married to that approach, I could add the hook more
here instead and be just as happy:

diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l
index 2412631..0ebf0c2 100644
--- a/Zend/zend_language_scanner.l
+++ b/Zend/zend_language_scanner.l
@@ -586,6 +586,9 @@ ZEND_API zend_op_array
*compile_file(zend_file_handle *file_handle, int type)
                if (compiler_result != 0) { /* parser error */
                        zend_bailout();
                }
+               if (zend_ast_hook) {
+                       zend_ast_hook(CG(ast));
+               }
                zend_compile_top_stmt(CG(ast));
                zend_ast_destroy(CG(ast));
                zend_arena_destroy(CG(ast_arena));

---

## #80760 — Stanislav Malyshev — 2015-01-18T21:20:14+00:00

URL: https://externals.io/message/80720#80760 · In reply to: #80720

Hi!


> https://github.com/php/php-src/pull/1004
> 
> Wondering if anyone has an objection to me merging this.  It just
> offers extensions a chance to alter the AST prior to bytecode

No objection but I wonder if it's time to have some list of the hooks -
especially in the light of the effort for making ZE API look like an API.

Also, I like "pre-processor hook" a bit better (second patch).
-- 
Stas Malyshev
smalyshev@gmail.com