Preserve get_called_class in directly compiled methods - #107
Conversation
matyhtf
left a comment
There was a problem hiding this comment.
Thanks for the focused fix and the thorough coverage of the normal inheritance/static-call cases. I found two blocking name-resolution issues that need to be addressed before this can be merged.
- The case-insensitive function-alias test does not verify the actual call target.
resolvedName is only used to decide whether to emit _typephp_called_class; the downstream lookup still uses $name / the case-sensitive useFunctions map. For example:
namespace ExternalFunctions {
function shadow(): string { return 'shadow'; }
}
namespace CalledAlias {
use function ExternalFunctions\shadow as GET_CALLED_CLASS;
class Probe {
final public function name(): string {
return get_called_class();
}
}
}PHP returns shadow, while this PR's compiled program returns CalledAlias\Probe. The new PHPUnit assertion only checks that _typephp_called_class is absent, so it passes even though runtime dispatch still targets the wrong function. Please use the resolved target consistently for compiled-function lookup, internal-function validation, and runtime dispatch, and make this an executable output test.
- The Native Class restriction remains case-sensitive and can be bypassed.
#[Native]
class Probe {
public function name(): string {
return \GET_CALLED_CLASS();
}
}This compiles successfully and then fails at runtime with “get_called_class() must be called from within a class”, instead of producing the existing Native Class compile-time diagnostic. PHP function names are case-insensitive, and the new special case already uses strcasecmp(), but the Native restriction runs earlier using exact equality. Please resolve/canonicalize the target once and use the same predicate for both policy validation and lowering.
Please also cover an unqualified namespaced call whose Ns\get_called_class() implementation becomes available through eval/include. Such a call is not definitively the global builtin; folding it directly to the called class changes PHP namespace fallback semantics.
Once target resolution is centralized and these cases have executable regression coverage, the overall lowering approach looks reasonable.
eb883de to
e899443
Compare
A final instance method on a concrete class can be called directly by generated
C++ code. If it calls
get_called_class(), the generated runtime function callhas no Zend method frame to inspect and raises “must be called from within a
class”, even though the same PHP program correctly returns the runtime class.
For zero-argument named calls resolved to this built-in in ordinary compiled
methods, reuse the receiver/called-scope machinery used by
static::class.Apply this after user-function resolution and argument validation. Use the
parser-resolved function target to distinguish case-insensitive aliases. Leave
dynamic calls, first-class callables, Native-class restrictions, and calls
outside methods on their existing paths. Do not expand devirtualization.
Validation: 149 focused PHPUnit tests / 302 assertions pass. A new compiled
PHPT matches PHP and EXPECT for inherited instance/static calls, traits,
mixed-case fully qualified names, aliases, closures, runtime subclasses,
namespaced function shadowing, and the error outside class scope. The minimal
unmodified-compiler repro exits 255; the fixed build matches PHP with empty
stderr. Tested on Linux ARM64, GCC O2, PHP 8.5.10 ZTS and PHPX 4b3a472.
This is a correctness fix with no claimed benchmark gain; it is not a general
solution for every runtime API that inspects Zend execution frames.