Skip to content

undefine user constants at runtime #433

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

Closed
wants to merge 2 commits into from
Closed

undefine user constants at runtime #433

wants to merge 2 commits into from

Conversation

krakjoe
Copy link
Member

@krakjoe krakjoe commented Sep 6, 2013

Seems like an omission ... any reason not to have it ??

@nikic
Copy link
Member

nikic commented Sep 6, 2013

-1 because:

  • This allows you to change constants (undef + def). Let me say that again: Change constants.
  • This will likely cause unexpected behavior, because we evaluate many constant-based values only once. E.g. given a property which defaults to a constant public $foo = FOO; this constant will only be evaluated once. Changing its value with undef+def will not change the default value of $foo. Same applies to any other place using static scalars.
  • ZEND_FETCH_CONSTANT currently uses our inline cache. This would no longer be possible and would thus slow down execution.
  • This particular patch does not respect optional constant case sensitivity.

@krakjoe
Copy link
Member Author

krakjoe commented Sep 6, 2013

  • doesn't seem that alien to me to change a constant, or to have #undef where there is #def
  • doesn't work on class constants, globals constants only
  • not sure about this one
  • fixed

@lt
Copy link
Contributor

lt commented Sep 6, 2013

I have to agree with @nikic. Constants that change are... well, they're variables aren't they.

Once defined, I would expect a constant to remain defined, and unchanged. Absolutely entirely unexpected behaviour if a constant value is able to vary.

What is your actual use case for this? I can only think of scenarios that would be better solved with a different approach entirely.

@nikic
Copy link
Member

nikic commented Sep 6, 2013

doesn't work on class constants, globals constants only

Doesn't really matter. For context see zend_update_constant_ex, which we call on all static scalar values on first use. The function will resolve all constants (be they global or class) and compute the resulting value. So constant-based values (in static_scalar contexts) will only fetch the constant once on first use, but never again. So changes in the constant value will not be reflected there.

not sure about this one

See the first few CACHED_PTR usages in http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#3489. If you undef a constant, the zend_constant for it is destroyed, so you'll leave a dangling ptr in the inline cache.

@krakjoe
Copy link
Member Author

krakjoe commented Sep 6, 2013

ok, good enough reasons to drop it ...

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.

3 participants