Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:74106
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain fb.com designates 67.231.153.30 as permitted sender)
To: Levi Morrison <levim@php.net>
CC: internals <internals@lists.php.net>
Thread-Topic: [PHP-DEV] [RFC] Return Type Declarations pre-vote follow-up
Thread-Index: AQHPa5iwbRN8uH8IdkWOuZ8Vefj2F5s5InuA
Date: Fri, 9 May 2014 20:06:30 +0000
Message-ID: <90B71511-4FB4-4916-9AC0-E3DD0D328C37@fb.com>
References: <CAFMT4Nrie2zAtVxcxg_xsDWs7EAa5fxk+d8aBH1=sZ=s_KFY2g@mail.gmail.com>
In-Reply-To: <CAFMT4Nrie2zAtVxcxg_xsDWs7EAa5fxk+d8aBH1=sZ=s_KFY2g@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <37E73FEB7309034C8A52BBC46DD2B8E8@fb.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC] Return Type Declarations pre-vote follow-up
From: jwatzman@fb.com (Josh Watzman)

On May 9, 2014, at 8:09 AM, Levi Morrison <levim@php.net> wrote:

> PHP Internals enthusiasts,
>=20
> It has been two weeks since I opened the discussion about adding return
> type declarations to PHP (for those who missed the prior discussion there
> are links at the bottom of this message). So far there has been
> overwhelmingly supportive feedback and a few minor issues were discovered
> and addressed. I thank everyone who has helped to make this RFC as good a=
s
> possible.
>=20
> As of yesterday I could have moved this RFC to vote; however, due to the
> announcement of the phpng branch and a few other things I worry that not
> everyone has had time to sufficiently review and comment on this RFC. As
> such I will delay for another week before moving this RFC into voting
> phase. If a major issue is discovered then it will be addressed before
> moving to vote.
>=20
> Again, I thank everyone who has participated so far and invite others to
> participate if they have been holding back.

Hey Levi et al! For those who don't know me, I'm Josh Watzman and I work on=
 the Hack team at Facebook. We're really excited to see PHP picking up some=
 of the features of Hack and would love to work with you all to ensure comp=
atibility as much as possible. Levi has been quite proactive in chatting wi=
th us about this one, which we appreciate. I only recently subscribed to in=
ternals, and apologize for my absence until now!

We looked over the RFC, as well as actually running the test suite against =
HHVM. This uncovered one important issue, two minor issues, and even one bu=
g in HHVM! I apologize if these have been brought up before -- I looked thr=
ough the archives and didn't see any discussion on the list about these, bu=
t could easily have missed something.

The important issue: I'd like to raise discussion on test rfc004.php (the "=
missing return type on override" example in https://wiki.php.net/rfc/return=
typehinting#examples). HHVM and Hack do not consider this to be an error. I=
n type system terms, we consider a missing annotation to be both a supertyp=
e and subtype of everything. This means that, not only can a superclass omi=
t an annotation that a subclass specifies, but a *subclass* can omit an ann=
otation that the superclass specifies. While this isn't the most principled=
 methodology from a hardcore type system perspective, we believe it is very=
 pragmatic and interoperates well with the dynamically typed world of PHP. =
It's been extremely important to us at Facebook that it was always 100% saf=
e to remove a type annotation and revert to old-style, untyped PHP -- the t=
ype system could never get in your way if you wanted to tell it to go away =
once and for all. There is always an easy escape hatch. If you require subc=
lasses to have an annotation if the superclass does, this principle is brok=
en -- local removal of the return type might introduce a different error si=
nce the subclass no longer matches the superclass! So I urge you to reconsi=
der this. Relaxing this doesn't seriously compromise the intent of the RFC,=
 but it does relax the constraints on those who want to only partially type=
 their code (which is extremely common at Facebook; we've spent a lot of ef=
fort on conversion and only about 40% of our code is type annotated even to=
day).

First minor issue: you have a return type "self". This is an LSB type, effe=
ctively the type of "new static". Levi, I remember discussing this with you=
 in IRC, but not the result of that discussion. What is the reason for "sel=
f"? The Hack type system calls it "this", which I admit is a little confusi=
ng since it is the type of more things than just the literal variable "$thi=
s" ("new static" for example is also of type "this"). But the type "static"=
 is much more consistent with the existing LSB language, i.e., "new static"=
.. So is there any reason to call it "self" over "static" or "this"?

Second minor issue: your reflection implementation is slightly different fr=
om HHVM's. I'm told we call the method "getReturnTypeText", which returns t=
he hint text, or false if none exists. I don't know how strongly we feel ab=
out this, but at the very least, I wanted to make sure you were aware that =
you were definitely diverging from the existing implementation here, and th=
at it was done intentionally. (Also, I don't think you have any test covera=
ge of this feature?)

Thanks for putting this together -- the FB team is really looking forward t=
o it.
Josh Watzman