Hi Larry, Thank you for your review.
> As you note, this RFC is a little more than erased generics; it does provide > a little validation, and reflection support (in addition to a standardized > syntax that's much more understandable than the current de facto standard > docblock syntax). That's not nothing, and is a marginal improvement over the > status quo. The question is whether that is enough of a benefit, and what > future improvements it makes easier/harder. I'd argue this RFC makes future improvements strictly easier, not harder. see https://wiki.php.net/rfc/bound_erased_generic_types#future_scope. The work this RFC does (syntax, parsing, compile time enforcement, reflection) is baseline infrastructure that any generics implementation in PHP will need regardless of approach. Landing it now gives us a foundation to build on. > Technically, anyone using Symfony or Laravel is "using generics" in that > Symfony and Laravel have generic doc-types on their code. That doesn't imply > that everyone building a site with Symfony or Laravel is regularly running > PHPStan to verify those, or adding their own doc-types to match it. I disagree here. Someone using a Laravel collection class without engaging with its `@template` annotations isn't "using generics", they're consuming a typed API and ignoring the type parameters. That pattern continues unchanged under this RFC: a user can call `$collection->map(...)` without ever writing or reading a generic type argument. The only place behavior changes is inheritance: someone who extends or implements a generic class is now required to provide type arguments (e.g. `class MyCollection extends Collection<int>`), and *that* get enforced at both compile time and runtime, because concrete type arguments get substituted into method signatures. > Where this becomes a land-mine is less the production deploys today, but that > future improvements become BC breaks. [...] My fear is that people who don't > use SA tools will write code on top of someone else's generic code, not care > that their types are buggy, not notice, and then we start enforcing it in the > future and their code breaks. This is the right concern to raise, and I think it's addressable without needing the "your types are wrong, not covered by BC" escape hatch you propose below. The principle: as long as we don't change the semantics of existing syntax, no future improvement introduces a BC break. Reified generics, for example, can be added later as opt-in, just like how HackLang did it (https://docs.hhvm.com/hack/reified-generics/reified-generics-migration/). A class or function would need to declare `#[ReifiedGenerics]` (or similar) to opt into reified semantics; everything else continues to behave exactly as the bound-erased model specifies. Library and framework authors then choose the strictness-vs-performance tradeoff that fits their use case, and existing code never breaks because the default behavior never changes. So the "BC only for sloppy code" risk you describe doesn't materialize, sloppy code today stays sloppy tomorrow at exactly the same level. > Another option, if we want to take the stance that SA is The Solution(tm) to > generics, is to ship an actual first-party SA tool with PHP. I want to push back on this strongly. speaking as Mago's author for a moment. The existing SA tools (PHPStan, Psalm, Mago) aren't just type checkers. Type checking is part of what they do, but they also handle a much larger surface: types PHP itself has no notion of (`positive-int`, `non-empty-lowercase-string`, `class-string<Foo>`, `list{1, 3, "hello"}`, and 100s more, see https://carthage-software.github.io/suffete/universe/elements.html), plus code quality rules, security analysis, dead code detection, and so on. A first-party tool would need to compete on that whole surface to be useful, anything narrower would disappoint the audience that actually wants SA. That leaves two options, neither good: 1. A weaker tool than what exists. This doesn't serve the audience that wants real SA, and users will rightly be frustrated that PHP shipped something less capable than third-party alternatives. 2. A tool competitive with existing ones. This is probably a year or two long C project requiring a dedicated team, on top of the PHP core team's existing scope. As someone who built a full toolchain in Rust from scratsh following Psalm/Hakana's footsteps, I can tell you this is substantially harder than it looks, and it would be *considerably* harder in our case than it was for me, for two reasons: 2.1. Mago is written in Rust, which is a more productive language for this kind of tool and has the ecosystem to match. Building the equivalent in C, against PHP's existing engine constraints, means a much higher cost per feature, per bug fix, per refactor. 2.2. Mago isn't part of PHP. If we get variance wrong, we ship a fix the same day. sometimes twice a day, sometimes once a week. We aren't bound by the php-src release cycle, and we iterate as soon as we have a fix. A first-party SA tool inherits PHP's release cadence: fixed-cadence releases, feature freeze windows, stability guarantees. For a tool whose value depends on keeping up with framework patterns, library conventions, and emerging idioms, that release shape is wrong. Beyond the engineering cost, there's a parser problem: PHP's current parser doesn't produce the full CST that an SA tool needs. You'd need a new parser, a static name resolver, a static reflection system that doesn't execute files to extract type information, and so on, a substantial reimplementation of analysis infrastructure in C, which the community already has high-quality versions of in Rust and PHP. In short: shipping a first-party SA tool would be a large, probably multi-year, investment for a result that's almost certainly worse than the tools the community has already built. I don't think it's a good use of PHP's resources, and I'd argue strongly against it. > So for me to vote in favor of this RFC, we would need to have *some* degree > of first-party enforcement I think the RFC already provides substantial first-party enforcement, more than the "erased" framing suggests. The engine validates arity at declaration and inheritance sites, bound conformance including bound-on-bound for forwarded parameters, variance soundness at declaration positions, parametric LSP into substituted method signatures, default-vs-bound conformance, the 127-arg cap, top-level self-reference and shadowing, and arity+bounds at every turbofish call site. The compile-time and link-time enforcement surface is comparable to Java's. What's not enforced is parametricity at call sites, which is the "erased" part. If the concern is "PHP should validate generic code itself, not require external tools," then this RFC does that, for everything the runtime can check without reified types. > I agree with Bob that the +/- syntax is very confusing. I would much rather > use in/out, as seen in Kotlin and C#, which are both vastly more > self-documenting and match what some of our peer languages are doing. No strong preference here. I went with `+`/`-` because that's what HackLang uses and it's familiar to me, but `in`/`out` is fine if that's the consensus. Happy to switch if enough people prefer the keyword form. > An interesting quirk I found in my previous research is that languages that > use : for inheritance use : for bounding generics. Languages that use > "extends" for inheritance also use "extends" for bounding generics. PHP uses > "extends", so that pattern would suggest we use "class Box<T extends FooBar>" > rather than :. I'd disagree on this one. `class A<T extends C> extends B` reads awkwardly to me, especially when the bound is scalar or a union eg., `class A<T extends int|string> extends B`, is rough to parse visually. The colon form keeps the type-parameter bound visually distinct from the class's own extension relationship, which I think is the more important consistency to optimize for. > I am completely OK with skipping typed arrays at this point. In practice I'd > rather build objects with nice operator overrides directly into the stdlib. Typed array deserve their own RFC. My initial draft did include `array<K, V>`, but without enforcing arity or bounds it's effectively useless, the array-key coercion problem makes this: ```php function x(array<string, string> $a, string $k, string $v): array<string, string> { $a[$k] = $v; return $a; } ``` potentially return an `array<string|int, string>` if `$k` is a numeric string. Properly typed arrays need separate, more careful design than this RFC can give them. > I fully expect "turbofish" to result in all kinds of slide shenanigans at > conferences. At least on my slides. :-) It hasn't been a problem in Rust, and I don't think it will be in PHP either. (Looking forward to the slides though) > What would a turbofish on a static call look like? self::<Bar>foo(1) or > self::::<Bar>foo(1)? The RFC should specify. Neither. the syntax is `self::foo::<Bar>(1)`. The turbofish always comes after the method name and before the argument list, consistent with how it's used in Rust. I'll make this explicit in the RFC. > I'd be completely OK with lowering the argument cap from 127, too. that makes sense in principle, but assuming you mean the actual-parameter cap rather than the type-parameter cap, that's a separate RFC and would itself be a BC break. Worth doing eventually, just not as part of this one. > Why is ReflectionGenericVariance backed? I don't see what the ints add. It's backed because the values match what the engine uses internally. no strong preference though, i can switch it to a unit enum if that reads better. > The "What is/isn't enforced" section could really use examples. A lot of it > is hard for me to follow as it's so abstract. Same for the Limitations > section. Examples please. fair. The PR's test suite has all the relevant behaviors covered, so I'll extract examples from there into both sections. > OK, having read the full RFC, it seems to be sort of "mostly-erased" > generics. There is notably more enforcement than the term "erased" would > imply. [...] It looks like, in practice, basically everything on the > declaration side is enforced; it's just the call side that is unenforced. Is > that an approximately accurate summary? Approximately, yes, declaration-side enforcement is substantial, call-side type arguments are erased. The proposal is closer to Java's model than Python's, but I want to keep the "erased" terminology because the defining property, type arguments not available at runtime, is what distinguishes this from reified generics. "Bound-erased" is the precise term: bounds are enforced where they can be (declaration sites, substituted method signatures), type arguments themselves are erased at use sites. That said, your point about expectations is fair, I'll consider moving the enforcement section higher in the RFC to set those expectations earlier. > I recommend making a bigger deal of the fact that the turbofish being > optional is a BC layer. Having the absence of it default to mixed is a *huge* > deal for making generics adoption smoother. True, this deserves more visibility. The whole point is to make adoption smooth: existing code doesn't need to be annotated all at once (or at all, since SA tools can already infer most of it), and incremental migration just works. I'll elevate this in the RFC. > If I understand correctly, this RFC would allow for foo(Collection<int> $c) > {}, but wouldn't actually enforce it at runtime. Within the function, > `instanceof Collection` would still work, but there's no `instanceof > Collection<int>` alternative. Am I reading that correctly? yes. that's exactly the erasure behavior. the type argument isn't available at runtime, so `instanceof Collection<int>` is the same as `instanceof Collection`. If you pass a `Collection<string>` to a function expecting `Collection<int>`, runtime accepts it because both are `Collection` at runtime; the mismatch is caught by SA, not by the engine. > I think at this point I am still skeptical, but warming to it, and could be > convinced. But more convincing is needed. And lunch, which I think I need > after reading all of this. :-) Hopefully the above moves you a bit further along. Please let me know if you have more questions or concerns. Cheers, Seifeddine.
