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

Unset from object property array not working #1259

Closed
baszczewski opened this issue May 23, 2016 · 7 comments · Fixed by #2222 or #2230
Closed

Unset from object property array not working #1259

baszczewski opened this issue May 23, 2016 · 7 comments · Fixed by #2222 or #2230
Assignees
Labels
Milestone

Comments

@baszczewski
Copy link
Contributor

baszczewski commented May 23, 2016

Unfortunately unset from array is not working.

Sample function code:

var_dump(this->_data);
unset this->_data["key_a"];
var_dump(this->_data);

compiled C code:

zval *key_param = NULL, _0, _1, _2;
zval key;
ZEPHIR_INIT_THIS();

ZVAL_UNDEF(&key);
ZVAL_UNDEF(&_0);
ZVAL_UNDEF(&_1);
ZVAL_UNDEF(&_2);

ZEPHIR_MM_GROW();
zephir_fetch_params(1, 1, 0, &key_param);

zephir_get_strval(&key, key_param);


zephir_read_property(&_0, this_ptr, SL("_data"), PH_NOISY_CC | PH_READONLY);
zephir_var_dump(&_0 TSRMLS_CC);
zephir_read_property(&_1, this_ptr, SL("_data"), PH_NOISY_CC | PH_READONLY);
zephir_array_unset_string(&_1, SL("key_a"), PH_SEPARATE);
zephir_read_property(&_2, this_ptr, SL("_data"), PH_NOISY_CC | PH_READONLY);
zephir_var_dump(&_2 TSRMLS_CC);
ZEPHIR_MM_RESTORE();

result:

array(2) {
  ["key_a"]=>
  string(6) "marcin"
  ["key_b"]=>
  string(5) "paula"
}
array(2) {
  ["key_a"]=>
  string(6) "marcin"
  ["key_b"]=>
  string(5) "paula"
}

Zephir version: 0.9.3a-dev
PHP: 7.0
OS: Ubuntu 16.04

May I provide additional informations, which help resolve this issue?
I tried to modify C code by myself . After remove PH_SEPARATE attribute, issue is gone.

@baszczewski baszczewski changed the title Unset from object property array not working (again) Unset from object property array not working Jun 17, 2016
@danhunsaker danhunsaker added this to Needs triage in Zephir Bugs Dec 12, 2018
@sergeyklay sergeyklay added the bug label Mar 10, 2019
@sergeyklay sergeyklay moved this from Needs triage to High priority in Zephir Bugs Mar 10, 2019
@sergeyklay
Copy link
Member

@dreamsxin Could you please take a look?

@Jurigag
Copy link
Contributor

Jurigag commented Jun 11, 2019

@dreamsxin as you can see above it's used within phalcon itself causing some problems

@dreamsxin
Copy link
Contributor

dreamsxin commented Jun 11, 2019

@sergeyklay zephir parse add object-property-unset, or variable add separate flag.
@Jurigag I test zephir/development it's correct.

$arrtest = new Test\ArrayAccessObj;
var_dump($arrtest);
unset($arrtest['one']);
var_dump($arrtest);

The Z_REFCOUNT_P(_zv) always equals 1, will skip SEPARATE_ZVAL_IF_NOT_REF.

#define SEPARATE_ZVAL_IF_NOT_REF(zv) do {				\
		zval *_zv = (zv);								\
		if (Z_COPYABLE_P(_zv) ||                    	\
		    Z_IMMUTABLE_P(_zv)) {						\
			if (Z_REFCOUNT_P(_zv) > 1) {				\
				if (!Z_IMMUTABLE_P(_zv)) {				\
					Z_DELREF_P(_zv);					\
				}										\
				zval_copy_ctor_func(_zv);				\
			}											\
		}												\
	} while (0)

Test2:

class ArrayAccessObj implements \ArrayAccess
{
	protected test;
	public test2;

	public function __construct() {
		let this->test = [
			"one":1,
			"two":2,
			"three":3
		];
	}
	
	public function setTest2(string! offset, value)
	{
		let this->test2[offset] = value;
		return this->test2;
	}
	
	public function unsetTest2(string! offset)
	{
		unset this->test2[offset];
		return this->test2;
	}
}
$arrtest = new Test\ArrayAccessObj;
//var_dump($a = $arrtest->test2 = ['test', 'test']);
var_dump($a = $arrtest->setTest2('one', 'one'));
var_dump($arrtest->setTest2('two', 'two'));
var_dump($arrtest->unsetTest2('one'), $a);

@Jurigag
Copy link
Contributor

Jurigag commented Jun 12, 2019

That's weird, because in this PR as you can see test is failing https://github.com/phalcon/cphalcon/pull/14176/files

@dreamsxin
Copy link
Contributor

@Jurigag Can use zephir development version test again, the change 843f5c8#diff-773a733807b93616c23cc11607b87a30R573

@ruudboon
Copy link
Contributor

I'm seeing that this issue still occurs in 0.12.2.

@sergeyklay
Copy link
Member

sergeyklay commented Sep 22, 2019

@ruudboon could you please send a pull request with a test shows the issue

@niden niden removed this from High priority in Zephir Bugs Feb 16, 2021
@Jeckerson Jeckerson added this to the 0.13.x milestone Apr 14, 2021
Jeckerson added a commit that referenced this issue Apr 19, 2021
@Jeckerson Jeckerson linked a pull request Apr 19, 2021 that will close this issue
3 tasks
@Jeckerson Jeckerson self-assigned this Apr 19, 2021
Jeckerson added a commit that referenced this issue Apr 19, 2021
…F() and SEPARATE_ARRAY()

In PHP8.1 this macro will be removed - php/php-src@ec58a6f
Jeckerson added a commit that referenced this issue Apr 24, 2021
Jeckerson added a commit that referenced this issue Apr 24, 2021
Jeckerson added a commit that referenced this issue Apr 24, 2021
Jeckerson added a commit that referenced this issue Apr 24, 2021
@Jeckerson Jeckerson modified the milestones: 0.13.x, 0.13.3 Apr 24, 2021
@Jeckerson Jeckerson mentioned this issue Apr 24, 2021
Jeckerson added a commit that referenced this issue Apr 24, 2021
@Jeckerson Jeckerson linked a pull request Apr 24, 2021 that will close this issue
3 tasks
Jeckerson added a commit that referenced this issue Apr 24, 2021
AlexNDRmac added a commit that referenced this issue Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants