From 7d260b7099469778113d458fccd765b7e5218a80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Grubbstr=C3=B6m=20=28Grubba=29?=
<grubba@grubba.org>
Date: Mon, 23 Mar 2015 20:56:54 +0100
Subject: [PATCH] Runtime: Fixed over optimization in F_APPEND_ARRAY.
Setters expect to be called when the corresponding variable is modified...
This behaviour caused Roxen's test-suite to fail due to change triggers
not being called.
---
src/array.c | 16 +++++++++++-----
src/testsuite.in | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/src/array.c b/src/array.c
index 067d5658eb..70da9c0257 100644
--- a/src/array.c
+++ b/src/array.c
@@ -448,6 +448,7 @@ void o_append_array(INT32 args)
{
struct svalue *lval = Pike_sp - args;
struct svalue *val = lval + 2;
+ int lval_type;
#ifdef PIKE_DEBUG
if (args < 3) {
Pike_fatal("Too few arguments to o_append_array(): %d\n", args);
@@ -455,7 +456,7 @@ void o_append_array(INT32 args)
#endif
args -= 3;
/* Note: val should always be a zero here! */
- lvalue_to_svalue_no_free(val, lval);
+ lval_type = lvalue_to_svalue_no_free(val, lval);
if (TYPEOF(*val) == T_ARRAY) {
struct svalue tmp;
@@ -464,10 +465,15 @@ void o_append_array(INT32 args)
element and do not do the assign. This can be done because the
lvalue already has the array as it's value.
*/
- if( v->refs == 2 )
- {
- if( v->real_item+v->malloced_size >= v->item+v->size+args )
- {
+ if( (v->refs == 2) && (lval_type != PIKE_T_GET_SET) ) {
+ if ((TYPEOF(*lval) == T_OBJECT) &&
+ lval->u.object->prog &&
+ ((FIND_LFUN(lval->u.object->prog, LFUN_ASSIGN_INDEX) >= 0) ||
+ (FIND_LFUN(lval->u.object->prog, LFUN_ASSIGN_ARROW) >= 0))) {
+ /* There's a function controlling assignments in this object,
+ * so we can't alter the array in place.
+ */
+ } else if( v->real_item+v->malloced_size >= v->item+v->size+args ) {
struct svalue *from = val+1;
int i;
for( i = 0; i<args; i++,from++ )
diff --git a/src/testsuite.in b/src/testsuite.in
index 91009427aa..e8ac2a8774 100644
--- a/src/testsuite.in
+++ b/src/testsuite.in
@@ -4112,6 +4112,52 @@ test_any([[
return has_index(a->_modified, "foo");
]], 1)
+test_any([[
+ // Since the addition of F_APPEND_ARRAY the setter is not being
+ // called anymore. Instead, the array _data is modified in place.
+ class A(array _data) {
+ int counter;
+
+ void `foo=(mixed v) {
+ counter += !!v;
+ _data = v;
+ }
+
+ mixed `foo() {
+ return _data;
+ }
+ };
+
+ object a = A(({}));
+
+ for (int i = 0; i < 6; i++) {
+ a->foo += ({ i });
+ }
+
+ return a->counter;
+]], 6)
+
+test_any([[
+ // Since the addition of F_APPEND_ARRAY the setter is not being
+ // called anymore. Instead, the array _data is modified in place.
+ class A(array foo) {
+ int counter;
+
+ mixed `->=(string sym, mixed v) {
+ counter += !!v;
+ return ::`->=(sym, v);
+ }
+ };
+
+ object a = A(({}));
+
+ for (int i = 0; i < 6; i++) {
+ a->foo += ({ i });
+ }
+
+ return a->counter;
+]], 6)
+
test_any([[
// Triggered fatal since object_equal_p did not handle
// getter/setter identifier correctly
--
GitLab