Skip to content

Preserve get_called_class in directly compiled methods - #107

Merged
matyhtf merged 3 commits into
swoole:masterfrom
yavon007:codex/direct-get-called-class
Sep 14, 2026
Merged

matyhtf merged 3 commits into
swoole:masterfrom
yavon007:codex/direct-get-called-class

Conversation

@yavon007

Copy link
Copy Markdown
Contributor

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 call
has 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.

@matyhtf matyhtf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. 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.

  1. 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.

@yavon007
yavon007 requested a review from matyhtf September 14, 2026 07:22
@yavon007
yavon007 force-pushed the codex/direct-get-called-class branch from eb883de to e899443 Compare September 14, 2026 07:27
@matyhtf
matyhtf merged commit df6c0df into swoole:master Sep 14, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants