Skip to content

Instantly share code, notes, and snippets.

@ircmaxell
Created July 25, 2014 21:35
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ircmaxell/6fa2ea9417056be7aba7 to your computer and use it in GitHub Desktop.
Save ircmaxell/6fa2ea9417056be7aba7 to your computer and use it in GitHub Desktop.
Dump of #php.pecl convo
13:11 <ajf> I’m mostly happy with it as-is, but given that I made int/float/numeric/string not accept NULL, I think booleans shouldn’t be accepted for them either.
13:19 → arty joined ⇐ DaveRand quit
13:49 → sonotos and lonnylot joined ⇐ dmitry and RemiFedor quit
14:41 <ircmaxell> ajf: that sounds sane (consistency wise). So it means that `int/float/numeric` only accept int/float/valid-number-string, and string only accepts string/int/float
14:43 <ajf> ircmaxell: Yeah
14:43 <ajf> Also, we don’t accept int/float/string for bool, so we shouldn’t accept bool for int/float/string
14:43 <ircmaxell> yeah, makes perfect sense
14:44 <ajf> Another point someone raised was that FALSE is a common error value alongside NULL.
14:44 <ajf> So, should I go ahead and amend the patch and RFC to do that?
14:44 <ircmaxell> I think that's the direction we're going, so that's valid
14:44 <ircmaxell> ++
14:45 <ajf> :)
14:54 ⇐ arty quit (~arty@178-26-90-22-dynip.superkabel.de) Quit: My MacBook Pro has gone to sleep. ZZZzzz…
15:00 <ajf> And done.
15:00 <ajf> Oh wait, oops
15:00 <ajf> OK, done
15:01 <ajf> ircmaxell: https://wiki.php.net/rfc/scalar_type_hinting_with_cast#conversion_rules
15:02 <ircmaxell> 12.5 -> int... I'm not 100% sold on that behavior...
15:03 <ajf> ircmaxell: Can you think of any instances where you might end up with 12.5 yet need it for an integer parameter?
15:03 <ircmaxell> division
15:03 <ircmaxell> foo($a / 2) where foo expects an integer
15:04 <ajf> ircmaxell: Cast explicitly or use intdiv()
15:04 <ajf> Granted, we haven’t added that yet ;)
15:04 <ircmaxell> well, you get what I mean though...
15:05 <ircmaxell> when would it obviously be an error to pass a float to something expecting an integer
15:05 <ajf> ircmaxell: It’s a bit of a minefield too because there are several approaches to conversion
15:05 <ajf> We could round or we could truncate
15:05 <ajf> Which does the user expect?
15:05 <ircmaxell> well, C always truncates, and I think that's the only valid response if we do accept it...
15:06 <ajf> ircmaxell: But then, what if I’ve ended up with 1.99999999999999999997 somehow?
15:06 <ircmaxell> very true
15:06 <ajf> I think in the cases where you might end up with a float, you should explicitly convert it to int yourself.
15:07 <ircmaxell> well, is there a good argument for not though?
15:07 <ircmaxell> meaning what cases would it cause serious issues?
15:08 <ajf> Hmm
15:08 <ircmaxell> because again, I think accepting "12.5" as int is a LOT safer than accepting "Apple". And saying "just cast" could introduce far more significant bugs... It's a tradeoff...
15:09 <ajf> True, if we make people cast, they lose the overflow protection
15:09 <ajf> Actually, PHP’s integer-float casting semantics are awful. It doesn’t handle NaN or Infinity right.
15:09 <ircmaxell> can it be cleanded up?
15:10 <ajf> It’s something my bigints RFC includes as a bit of a bonus, actually
15:11 <ajf> ircmaxell: Thing is, I don’t think you’re going to end up doing foo_takes_int(12.5) if your code is well-written
15:11 <ircmaxell> well, remember we're not targetting well-written code. We're targetting generic code
15:11 <ircmaxell> the average PHP user doesn't produce well-written-code from a type perspective. They expect it to "just work".
15:12 <ircmaxell> and that's something we need to balance. Making it "just work" where possible, and error where necessary.
15:12 <ajf> Right
15:13 <ajf> Division is the obvious case where you might end up with a float, but if you want integer division, you should be casting or rounding, or using intdiv() if that gets in.
15:13 <ircmaxell> and I'm not sure if 12.5 -> int conversion erroring is necessary enough to justify the error...
15:13 <ircmaxell> ajf: the average user doesn't know what integer division is
15:13 <ircmaxell> substr($foo, 0, strlen($foo) / 2) <--
15:14 <ajf> ircmaxell: Maybe not, but they probably do know about floor(), ceil() and round(), *especially* if they’ve ever touched JavaScript, which I’d say is quite likely
15:15 <ircmaxell> that works 100% today, and if we're talking about re-normalizing ZPP to these rules down the road, that's a problem...
15:15 <bwoebi> Tyrael: I thought the bugs were fixed now?
15:17 <ajf> ircmaxell: I wonder if allowing 12.5 might cause confusion. If the function just eats it without complaint, maybe someone might think the function actually accepts float parameters, and gets surprised later when they discover it doesn’t
15:18 <ircmaxell> I don't know if that's true, as people know they can pass a float to `substr`, but that doesn't mean they think it accepts floats
15:18 <ircmaxell> I mean it may cause some slight confusion, but nothing that isn't there today
15:18 <ajf> ircmaxell: Do they actually know that? I’m not actually sure how well zpp’s behaviour is documented
15:19 <bwoebi> I’m not sure if we even should make a difference between integer and float in the typehint?
15:19 <ajf> bwoebi: ?
15:19 <ircmaxell> bwoebi: that's what I'm thinking. Both should accept the same values, and what comes out on the other side is either an int or float (depending on what you asked for)
15:19 <bwoebi> ircmaxell: this.
15:20 <ircmaxell> so int/float/numeric both accept int/float/numeric-string, and return int/float/best-fit
15:20 <ajf> Hmm
15:21 <ajf> On the other hand, might that mask bugs? Bear in mind that not everything does treat floats and ints th
15:21 <ajf> e same
15:21 <ircmaxell> so really, it would look like: if (!is_numeric($foo)) { error("Expecting {type}, got $foo") } $foo = (type) $foo;
15:21 <ircmaxell> ajf it may mask bugs, but people casting to int may mask more (as it's an explicit cast)
15:22 <ajf> True
15:22 <@Tyrael> bwoebi: nevermind, I've somehow missed https://github.com/php/php-src/commit/c49a06168ed3759779452e2b2a44da22e75a0702 and https://github.com/php/php-src/commit/d909b6330e314bb434350317fda5bec185ea12c0
15:22 <bwoebi> good
15:23 → arty joined (~arty@178-26-90-22-dynip.superkabel.de)
15:24 <@Tyrael> on the scalar type coersion, I start to think that we should first unify the userland casting rules and the zpp arg parsing stuff (obviously in a major) to be able to introduce it
15:24 <ircmaxell> ajf: also, hold off on sending a mail for every revision... that thread is already way too long to live
15:25 <@Tyrael> currently whatever we pick, it will be inconsistent in some way or another with other parts
15:25 <ircmaxell> Tyrael: I think the approach we were looking at, is come up with a sane set of consistent rules that make sense, get them in for userland, then refactor ZPP to match
15:25 <ajf> ircmaxell: I’d rather keep internals informed, though :/
15:26 <ircmaxell> ajf: yes, keep them informed by sending a mail with overall changes, in a new thread once the concept is in place
15:26 <bwoebi> ajf: I agree with ircmaxell, you should now first work up every issue and then make a new thread, otherwise nobody will read it.
15:26 <ircmaxell> What we're doing at this point is tweaking the philosophy for dealing with these changes, so piecemeal decisions (like disallowing bool for numerics) doesn't really communicate that
15:27 <ircmaxell> instead, once the RFC is consistent and presents the clean story, share that story back (based off of feedback from the prior discussion)
15:27 <ajf> Ah, I see what you’re saying
15:28 <ircmaxell> Tyrael: the reason for userland-first, is that it avoids the bias of massive BC concerns, and let's us discuss what should happen in an ideal way, and then apply that ideal to zpp. The other way around *may* be much more difficult... Disagree? Or...?
15:29 <@Tyrael> ircmaxell: didn't thought of it that way
15:29 <@Tyrael> makes sense, but it changes the premise of the original rfc
15:29 <@Tyrael> which was to copy/follow the otherwise known rules of typejuggling in php
15:29 <ajf> Yeah, the introduction doesn’t make sense now
15:29 <ircmaxell> well, copy/follow where it makes sense
15:30 <ircmaxell> which is one reason for the numeric discussion (allowing 12.5 to an int typehint by truncating)
15:30 <@Tyrael> instead of that, we would introduce a new set of rules to cast scalars which can be more sensible
15:30 <ircmaxell> yeah, that may be better for the overall story
15:30 <ajf> Part of the problem with type juggling is that “scalar” is a fairly broad descriptor
15:31 <@Tyrael> but basically we would expect people to memorize yet another conversion table
15:31 <@Tyrael> because $foo = (scalar_type)$foo; would still be there
15:31 <ajf> Unlike with ==, these rules only cover a small set of useful cases, so I think they’re easier and more intuitive to learn
15:32 <ircmaxell> Well, I think there should be 2 sets of rules (and only 2). Implicit (where we don't know intent) and Explicit (where we do)
15:32 <ircmaxell> and none of the ZPP rules are documented... So the only people that *really* know them are internals developers (and those devs who know how to read the ZPP code)
15:32 <@Tyrael> ircmaxell: as far as I can tell we already have 2 in a sense
15:33 <@Tyrael> implicit cast array to string will produce a notice for example
15:33 <ajf> Well… we have more than 2 sets
15:33 <ajf> Numeric strings aren’t consistently interpreted.
15:33 <@Tyrael> but the scalar typehints would be different from the currently existing implicit and explicit casting rules
15:33 <ircmaxell> currently, we have 4 sets of rules. Explicit, ZPP, ==, and +
15:34 <ajf> Then there are array indices
15:34 <ajf> and functions which do it themselves
15:34 <@Tyrael> yeah, so introducing yet another similar but still different ruleset
15:34 <@Tyrael> which cannot be introduced with one short sentence
15:35 <ircmaxell> Tyrael: would you feel better if ZPP was refactored to match within the RFC?
15:35 <ircmaxell> as opposed to doing it in a later RFC?
15:35 <@Tyrael> I think we can't do that without merging some of those 4 rulesets
15:35 <@Tyrael> ircmaxell: yeah, this is what I was trying to say with "on the scalar type coersion, I start to think that we should first unify the userland casting rules and the zpp arg parsing stuff (obviously in a major) to be able to introduce it"
15:35 <ajf> More than 4. There are several casting rulesets.
15:36 <@Tyrael> but this is just my 2 cents
15:36 <@Tyrael> I just don't think we can have consistency without following exactly one of the already existing rulesets
15:36 <ircmaxell> well, I don't think Explicit and == can go away or change... + becomes a challenge, because context matters, so I don't think that'll go away. And I don't think ZPP should be merged into any of the 3
15:37 <ajf> ZPP’s always been different to userland functions, which I don’t like, but it’s the reality of things
15:37 <ircmaxell> Tyrael: if done correctly (forget what that is for a second), do you think you could support changing the ZPP rules to be more consistent?
15:37 <@Tyrael> and I can see why do you think that there is neither of the current ones are suitable for an explicit typehint
15:37 <ajf> Userland functions don’t enforce the number of parameters, userland functions error instead of returning NULL, etc.
15:37 <@Tyrael> ircmaxell: in a major I think I could
15:37 <@Tyrael> I mean we have more control over that
15:38 <@Tyrael> it could cause less BC impact for userland
15:38 <ircmaxell> yeah, talking major only at this point (since changing zpp is way out of the question for a minor)
15:38 <@Tyrael> as extensions could chose to keep the old behavior
15:38 <@Tyrael> via adjusting their zpp usage
15:39 <ircmaxell> well, if we change the internal rules ZPP uses, they'd need to re-implement ZPP to match the old way (or partially at least)?
15:39 <ajf> Just add more characters! /s
15:39 <ircmaxell> and there's nothing stopping avoiding using ZPP (or with "z"), just like there's no avoiding not hinting (foo($int) { if (!is_int($int)) {...
15:40 <ircmaxell> hmm...
15:41 <ircmaxell> ajf: what do you think about changing ZPP in this RFC (or creating a new one, since it's a different tactic) to match the concept we've been working towards? Make it a "Scalar Typing" RFC rather than type hint (as it's both internal and external)?
15:42 ⇐ arty quit (~arty@178-26-90-22-dynip.superkabel.de) Quit: My MacBook Pro has gone to sleep. ZZZzzz…
15:42 <ajf> I’m not sure
15:42 <ajf> I wonder if fixing PHP’s inconsistent and varying type conversion rules is a lost cause
15:43 <ircmaxell> it may be
15:44 <@Tyrael> ajf: btw. I think Stas come to the same conclusion regarding the current ruleset as me
15:44 <ircmaxell> but so far, in all the years I've been following internals, I can't remember any serious proposals to do so (there were a few that wanted to make everything purely strict), but nothing that tried to grasp the idealology of PHP...
15:45 <ajf> The problem is PHP doesn’t have an ideology
15:45 <ajf> I’m not sure anyone has exactly the same idea of what PHP is
15:45 <ircmaxell> ajf: yes, it does (when it comes to typing at least). "12" **is** int(12)...
15:46 <ajf> Except when it’s not
15:46 <ajf> PHP is (sadly?) not Perl
15:46 <ajf> In Perl you can’t easily distinguish “12” and 12 AFAIK. The same is hardly true of PHP, where they’re sometimes but not always equivalent
15:46 <ircmaxell> there's only one case that I know of where "12" isn't replaceable for int(12) (and vise versa), and that's in ctype_digit()
15:47 <ircmaxell> well, all the ctype apis...
15:47 <ajf> andreas-air:php-src ajf$ sapi/cli/php -r 'var_dump(23 ^ 7); var_dump("23" ^ "7");'
15:47 <ajf> int(16)
15:47 <ajf> string(1) ""
15:47 <ircmaxell> fair point...
15:48 <ajf> Also, hexadecimal number strings are sometimes, but only sometimes, considered numbers
15:48 <ajf> http://3v4l.org/GF9EI
15:48 <ajf> Is NULL an empty array? Sometimes, maybe. Sometime it isn’t.
15:48 <ircmaxell> well, that's fixable by making the string->int parsing rules consistent in all cases (I don't think there's justification for it changing, is there?)
15:48 <ircmaxell> null an empty array?
15:49 <ajf> $x = NULL; $x[2] = 3; var_dump($x); => array(1) { [2]=> int(3)}
15:50 <ircmaxell> well, that's not really casting going on...
15:50 <ircmaxell> it's transparent initialization, but I see the point
15:52 <ajf> I’m not sure if the RFC is actually sane or not. In some respects I think its conversion rules are sane, but isn’t insane to have so many sets of conversion rules in PHP? Then again, isn’t it insane already? Is having a sane set and an insane set insane, or is it saner than just having an insane set? I’m not sure.
15:53 <auroraeos> maybe we need a conversion rule rfc (slightly being a troll but serious as well) - another set will make the language even more ... schizophrenic
15:53 <ircmaxell> which is why I'm wondering if doing ZPP at the same time as user-land hints, making them all sane conversions, would be the better way...
15:54 <ajf> Yes, but then we just bikeshed about the right way to do userland hints.
15:54 <ajf> er
15:54 <ajf> ZPP hints
15:55 <ircmaxell> Compromise isn't always a good thing, and sometimes let the bikeshed happen without getting sucked into it. If the rules are sane enough, the vote should be clear. If they are not, then they are not...
15:55 <ajf> ircmaxell: Something I’d like to try is holding an online poll of sorts to see what people’s voting intentions are. I have no idea how people would vote atm.
15:55 <ircmaxell> ajf: I don't think people really know what they would vote on
15:56 <ircmaxell> The overall RFC has changed a number of times, and the discussion has been hard to follow
15:59 <ajf> :/
16:12 <ajf> I’m not sure where to go from here
16:15 → druid joined (~druid@lunetics.com)
16:15 <auroraeos> perhaps get it to the point where you're truly happy with it - and do a summary on list of the RFC in it's final form
16:15 <auroraeos> then attempt a short discussion for minor things
16:23 <ajf> I’m quite happy with it
16:23 <ajf> I’m just not sure it could pass
16:24 <bwoebi> quite. Yeah. But it still has its uncertainities…
16:37 <ajf> ircmaxell: I wonder if it might be worthwhile just changing the RFC to do exactly what zpp does, no exceptions
16:37 <ircmaxell> and then a later RFC to change the overall behavior?
16:37 <ajf> Yeah.
16:37 <ircmaxell> that's likely the most likely route to actually pass
16:38 <ajf> RIght
16:38 <ajf> Every little exception to and inconsistency with zpp is a problem
16:38 <ajf> The best solution is to have absolutely none and exactly emulate - no, use the same code - as zpp
16:38 <ajf> Obviously it’s an E_RECOVERABLE_ERROR and not a return NULL, but otherwise the same
16:38 <ircmaxell> at this point, while I don't think it's good to ship those rules, I agree that's about the only way forward...
16:39 <ajf> Right\
16:39 <ircmaxell> so +1 from me
16:39 <ajf> I don’t like zpp’s rules, but at least that would be consistent
16:41 <ircmaxell> and it would cast...
16:41 <ircmaxell> so it wouldn't be purely strict
16:42 <ajf> Right
16:43 <ajf> Actually, there’d need to be another difference from zpp: How do we handle nullability?
16:43 <ircmaxell> how does ZPP handle it?
16:43 <ajf> I’m not sure. Does it even have nullable types? *checks*
16:43 <ircmaxell> by my original behavior
16:44 <ircmaxell> if it's not nullable, you can never get null (null auto-casts), if it is nullable, then you can get null
16:44 <ircmaxell> http://lxr.php.net/xref/PHP_5_2/Zend/zend_API.c#327
16:44 <ircmaxell> at least depending on the type
16:45 <bwoebi> 5.2?
16:45 <ircmaxell> so it's only supported for strings
16:45 <ircmaxell> numbers can't be nullable
16:45 <ajf> PHP 5.2?!
16:45 <ircmaxell> whoops: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#check_null
16:46 <ircmaxell> I had referenced 5.2 yesterday, so search defaulted to 5.2 (even though I was browsing trunk)
16:46 <ircmaxell> but the behavior is the same
16:49 → cjones_ joined (~Adium@inet-hqmc03-o.oracle.com)
16:52 <ircmaxell> also: if you're going to do that (make scalar params follow zpp), I'd suggest explicitly targetting NEXT.0.0, as opposed to 5.NEXT...
16:53 <ajf> hmm
16:53 <bwoebi> agree. It’d be a too big BC break for 5.NEXT (at least if people consider engine_exceptions too big for 5.NEXT too)
16:56 <ajf> There’d be no BC break
16:57 <ajf> The problem is that you now have even more code that’ll break if you change zpp’s behaviour
16:58 <ircmaxell> exactly
16:58 <ircmaxell> if you release it in 5.NEXT, it basically prevents ZPP refactors down the road in NEXT
17:00 <ajf> Welp.
17:01 → EstaTiC joined ↔ cjones_ and domeh nipped out
17:05 <ajf> ircmaxell: Should I just withdraw the RFC as it stands? Amending it to instead propose doing the ZPP thing in 6 might be bad.
17:05 <ajf> Er, NEXT :)
17:05 <ircmaxell> yeah, that's fair, it preserves history.
17:05 <ircmaxell> HOWEVER, I woulnd't officially withdraw it
17:05 <ircmaxell> until you have the other one drafted
17:05 <ajf> Ah, I see
17:06 <ircmaxell> that way it isn't "we're giving up" but "Rather than do it that way, let's take a different approach"
17:06 <auroraeos> +1
17:06 <auroraeos> and make sure you link to the old from the new :)
17:06 <cjones_> and vice versa
17:07 <cjones_> And include the correct URL in ALL email about the RFC
17:07 <ircmaxell> ++
17:08 <ircmaxell> and title it so it's dead obvious "User Hints - ZPP Style" or something along those lines
17:08 <ajf> Right, this all sounds workable
17:08 <ajf> However I’m a little fed up of scalar type hints for today
17:08 <ajf> Maybe later.
17:09 <@bjori> parameter casting
17:10 <ircmaxell> ajf: I think this was quite productive. Thank you for taking point on it, you've done an amazing job on it so far. :-)
17:10 <ajf> Thanks ^^
17:10 <ajf> bjori: internal-style scalar parameters?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment