Bug 75921: consistancy when creating stdObject for empty values #3109
Conversation
Zend/zend_execute.c
Outdated
| zval_ptr_dtor_nogc(container); | ||
| object_init(container); | ||
| if (type != BP_VAR_UNSET) { | ||
| if (!make_real_object(container)) { |
There was a problem hiding this comment.
I presume the logic in this function was duplicated here because the first branch in it should always be taken in this case. If that's true, then perhaps just adding a zend_error to the original code would be best:
zval_ptr_dtor_nogc(container);
object_init(container);
+ zend_error(E_WARNING, "Creating default object from empty value");If that's not the case, then the logic would look cleaner without a goto jump:
if (type == BP_VAR_UNSET || !make_real_object(container)) {
There was a problem hiding this comment.
We could just as easily add the error here as well, but I went with the reuse the existing function route. I assume this object_init() branch here existed before the function existed, probably explains the bug report. But yes, I mind-farted there, and will apply that fix.
Add some consistancy when warning about creating a stdObject against an empty LHS variable. The warning was not being properly reported when doing a FETCH_OBJ_W.
|
I was going to say that this kind of change needs an RFC at first, but after looking at the details of the current behavior, I think this change can just go into 7.4. Right now, this warning is always generated for "first-level" operations (with their own opcodes) regardless of whether it's a read or write operation. The warning is not thrown in cases where we don't have a dedicated opcode and go through FETCH_W/RW instead. The behavior would make some kind of sense if it was separated for read-write vs write operations, but as this is not the case ( |
| return 0; | ||
| } | ||
| object_init(object); | ||
| zend_error(E_WARNING, "Creating default object from empty value"); |
There was a problem hiding this comment.
It would be better to replace the use of make_real_object_rw with make_real_object instead. If you look at that function it has a bunch of special handling to avoid some edge-case memory safety issues, and it's best if we can just reuse it.
|
I went ahead and rebased this myself. Now merged as e63febb into 7.4+. Thanks, and sorry for the delay :) |
Bug #75921
Add some consistancy when warning about creating a stdObject against an
empty LHS variable. The warning was not being properly reported when
doing a FETCH_OBJ_W.