Fix GH-12695: skip __get() when __isset() materialised the property

[nicolas-grekas]

Fixes #12695.

After __isset() returns true under ?? or empty(), the engine re-checks the property table before calling __get(). When __isset() has materialised the property (a pattern used by lazy proxies), its value is returned directly and __get() is skipped.

isset() is unchanged (it never called __get()).

Tests under Zend/tests/magic_methods/gh12695*.phpt cover:

• the fix for ?? and empty()
• non-regression: when __isset() doesn't materialise, __get() is still called and ??'s null-check still applies
• isset()'s unchanged behaviour

[iluuu1994]

Looks functionally correct. I'd be happy merging this to master if the rest of internals are happy.

[arnaud-lb]

This looks right to me otherwise

[arnaud-lb]

Nit: for clarity I suggest to add a ZEND_UNREACHABLE() branch here:

Suggested change

	}
	} else {
	ZEND_UNREACHABLE();
	}

[nicolas-grekas]

I checked and WRONG_PROPERTY_OFFSET can actually reach this point: the magic-isset block only filters on BP_VAR_IS and __isset being defined, not on the offset type, and IS_WRONG_PROPERTY_OFFSET is handled later in the call_getter branch (around line 1003). Adding ZEND_UNREACHABLE() here would assert in debug builds whenever someone uses $obj->privateField ?? $y on a class with __isset defined. So leaving the fall-through to retval = &EG(uninitialized_zval); as-is, which is correct for that case.

[iluuu1994]

Thanks! LGTM. Given this is on verge of bug and feature, it would be good to post on the list again in a separate thread about this PR, and clarify that this is proposed to be merged unless there are objections. We usually wait for at least 2 weeks for such changes to be merged. If there are concerns, this will need to go through the RFC process nonetheless.

[TimWolla]

Only looked at the tests and NEWS/UPGRADING. I trust the implementation review of Arnaud and Ilija.

[TimWolla]

Should this also test the case where __isset() materializes to null for completeness?

[TimWolla]

Suggested change

	class C {
	public function __get($n) {
	echo " __get($n)\n";
	return 'from-get';
	}
	public function __isset($n) {
	echo " __isset($n)\n";
	return true;
	}
	}
	echo "1) `??` when __isset=true and __get returns a value: __get is called:\n";
	echo "1) `??` when __isset=true and __get returns a value: __get is called:\n";
	class C {
	public function __get($n) {
	echo " __get($n)\n";
	return 'from-get';
	}
	public function __isset($n) {
	echo " __isset($n)\n";
	return true;
	}
	}

Minor formatting nit: The other cases define the class after the echo.