# Follow-up to STH user experience, this time with actual testing - Thread: https://externals.io/message/83940 - Original archive: https://news-web.php.net/php.internals/83940 - Started by Matthew Weier O'Phinney on 2015-02-27T00:29:58+00:00 - Messages: 12 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #83940 — Matthew Weier O'Phinney — 2015-02-27T00:29:58+00:00 URL: https://externals.io/message/83940#83940 I've taken some time the last couple days to compile both the Scalare Type Hints v0.5 (heretofor STHv5) and Coercive Scalar Type Hints (heretofore STHcoerce) patches and test some code against them. In each case, I modified the `Phly\Http\Response` class from my phly/http package to add scalar type hints, remove previous argument validation, and then ran the unit tests. Here's my details of the experience, and analysis. ### STHv5 With STHv5, my tests failed out of the box. First, I needed to add an error handler that would convert the `E_RECOVERABLE_ERROR` to an InvalidArgumentException so that I would get useful error messages from PHPUnit. Next, once I had done that, I had tests that were throwing invalid input at methods, and validating that the invalid arguments were caught. This worked in all but one specific case: passing a float value to an argument expecting an integer. In this particular case, the value was coerced to an integer, albeit lossily, and no error was raised. When I changed my test case to operate under strict_types mode, the test executed as I had originally expected, and succeeded. #### Analysis I did not expect the float value to be coerced, particularly as it had a fractional part. Yes, I understand that this is how casting _currently_ works in PHP, and that the patch uses the current casting rules. However, I'd expect that any float with a fractional value would not be cast to an integer; doing so is lossy, and can lead to unexpected results. The strict_types mode operated how I would expect it, but meant I was forced to add the declaration to the top of the file. Which leads to this: My tests operated differently in strict vs normal mode. That means my code does, too. Testing both modes would be difficult to do in an automated fashion; essentially, you're left needing to choose to support one mode or the other. This goes back to the point I was making earlier this week: I worry that having two modes will lead to a schism in userland libraries: those that support strict, and those that do not; there will be little possibility of testing both. ### STHcoerce With STHcoerce, my tests also failed out of the box, but not for the same reason. Instead, I had a bunch of errors reported based on code that PHPUnit was executing! In this case, I discovered that: - PHPUnit passes a boolean false to `debug_backtrace()`... which is documented as expecting an integer! (There are actually several constant values it accepts, all of which are integer values.) In this case, PHPUnit is relying on the fact that the engine casts booleans to the integers 0 and 1. (Zeev has written to the list already indicating that this coercion path will be supported in the patch.) - PHPUnit is passing the results of $reflector->getDocComment() blindly to `substr()` and preg_match*(). getDocComment() is documented as returning EITHER a string OR boolean false. Again, PHPUnit is relying on PHP to cast boolean false to an empty string. (Zeev has also indicated this coercion path may be re-introduced.) I was able to fix these in a matter of minutes, and then my tests ran, and passed without any changes. #### Analysis STHcoerce worked about how I expect STHv5 will work _if_ _every_ _file_ were declared in strict_types mode. In other words, it uncovered errors in calling both userland and internal functions. Userland typehints worked as I expected, with floats using fractional values not casting to integers. ### Wrap Up STHcoerce was actually far more strict than I found strict_types mode to be in STHv5. The reason is that it's a single, non-optional mode. This poses a potential challenge to migration, as noted in my problems using PHPUnit (Yes, I WILL be sending patches their way soon!): method calls and arguments that previously worked because of how PHP juggles types often do not work, particularly when going from boolean to other scalar values. However, the patch does what I'd expect in terms of preventing operations that would result in either data loss or data additions, all of which can have unexpected side effects if you don't understand the casting rules currently. The flip side to this is that you do not need to make any changes to your code in order to locate and fix errors. What this means from a library/framework author perspective is that, if the STHcoerce patch is merged, I can test against PHP 7, make fixes, and my code is not only forward-compatible, but also backwards to the various PHP 5 versions I might already be supporting — and that code now benefits from being more correct. Because STHv5 is opt-in only, the only way to see similar type errors is to somehow automate the addition of the strict_types declaration to all files in your project. While it can be done with tools like sed and awk, realistically it means that existing libraries cannot and/or will not benefit easily from the changes until they support PHP 7 as a minimum version. Additionally, it poses potential problems in terms of different testing results based on which mode you run in, leading to ambiguity for users. The only way to address the latter as a library author is to specify which mode your code is known to run in. Ironically, I'm seeing STHcoerce as more strict than STHv5 in terms of type handling, due to the fact that every call, userland or internal, is affected. The fact that it's always on makes it simpler for me to test against it now, and makes it easier to make my existing code both forward-compatible and more correct — even if I do not add type hints until I update my minimum supported version to PHP 7. I am slightly worried about the amount of work needed to make code run from v5 to v7 if STHcoerce is adopted. That said, the issues I found in PHPUnit were corrected in minutes (though a more thorough patch that allows early return from methods if doc comments are not strings would be better), and my own library's tests ran without problems once the PHPUnit issues were corrected (both code with and without typehints). Strict_types is definitely interesting, but I found that strict_types mode was more or less how I wanted code to operate, and to opt-in to that means changes to every file in my library — which is a much larger migration problem. In the end: kudos to everyone who is working on these patches and RFCs. I'm excited about scalar type hints in PHP 7! -- Matthew Weier O'Phinney Principal Engineer Project Lead, Zend Framework and Apigility matthew@zend.com http://framework.zend.com http://apigility.org PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc --- ## #83942 — Trevor Suarez — 2015-02-27T00:45:39+00:00 URL: https://externals.io/message/83940#83942 · In reply to: #83940 Thank you so much for taking the time to do this. Your explanations of a more "real-world" example are extremely valuable and provide a very strong hint at the effects that these RFC implementations may have, both in the short and long term. Seriously, very appreciated. On Thu, Feb 26, 2015 at 7:30 PM Matthew Weier O'Phinney wrote: > I've taken some time the last couple days to compile both the Scalare Type > Hints > v0.5 (heretofor STHv5) and Coercive Scalar Type Hints (heretofore > STHcoerce) > patches and test some code against them. > > In each case, I modified the `Phly\Http\Response` class from my phly/http > package to add scalar type hints, remove previous argument validation, and > then > ran the unit tests. Here's my details of the experience, and analysis. > > ### STHv5 > > With STHv5, my tests failed out of the box. First, I needed to add an error > handler that would convert the `E_RECOVERABLE_ERROR` to an > InvalidArgumentException so that I would get useful error messages from > PHPUnit. > Next, once I had done that, I had tests that were throwing invalid input at > methods, and validating that the invalid arguments were caught. This > worked in > all but one specific case: passing a float value to an argument expecting > an > integer. In this particular case, the value was coerced to an integer, > albeit > lossily, and no error was raised. > > When I changed my test case to operate under strict_types mode, the test > executed as I had originally expected, and succeeded. > > #### Analysis > > I did not expect the float value to be coerced, particularly as it had a > fractional part. Yes, I understand that this is how casting _currently_ > works in > PHP, and that the patch uses the current casting rules. However, I'd > expect that > any float with a fractional value would not be cast to an integer; doing > so is > lossy, and can lead to unexpected results. > > The strict_types mode operated how I would expect it, but meant I was > forced to > add the declaration to the top of the file. Which leads to this: > > My tests operated differently in strict vs normal mode. That means my code > does, > too. Testing both modes would be difficult to do in an automated fashion; > essentially, you're left needing to choose to support one mode or the > other. > This goes back to the point I was making earlier this week: I worry that > having > two modes will lead to a schism in userland libraries: those that support > strict, and those that do not; there will be little possibility of testing > both. > > ### STHcoerce > > With STHcoerce, my tests also failed out of the box, but not for the same > reason. Instead, I had a bunch of errors reported based on code that > PHPUnit was > executing! In this case, I discovered that: > > - PHPUnit passes a boolean false to `debug_backtrace()`... which is > documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is > relying > on the fact that the engine casts booleans to the integers 0 and 1. > (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) > - PHPUnit is passing the results of $reflector->getDocComment() blindly to > `substr()` and preg_match*(). getDocComment() is documented as returning > EITHER > a string OR boolean false. Again, PHPUnit is relying on PHP to cast > boolean > false to an empty string. (Zeev has also indicated this coercion path > may be > re-introduced.) > > I was able to fix these in a matter of minutes, and then my tests ran, and > passed without any changes. > > #### Analysis > > STHcoerce worked about how I expect STHv5 will work _if_ _every_ _file_ > were > declared in strict_types mode. In other words, it uncovered errors in > calling > both userland and internal functions. Userland typehints worked as I > expected, > with floats using fractional values not casting to integers. > > ### Wrap Up > > STHcoerce was actually far more strict than I found strict_types mode to > be in > STHv5. The reason is that it's a single, non-optional mode. This poses a > potential challenge to migration, as noted in my problems using PHPUnit > (Yes, I > WILL be sending patches their way soon!): method calls and arguments that > previously worked because of how PHP juggles types often do not work, > particularly when going from boolean to other scalar values. However, the > patch > does what I'd expect in terms of preventing operations that would result in > either data loss or data additions, all of which can have unexpected side > effects if you don't understand the casting rules currently. > > The flip side to this is that you do not need to make any changes to your > code > in order to locate and fix errors. What this means from a library/framework > author perspective is that, if the STHcoerce patch is merged, I can test > against > PHP 7, make fixes, and my code is not only forward-compatible, but also > backwards to the various PHP 5 versions I might already be supporting — > and that > code now benefits from being more correct. > > Because STHv5 is opt-in only, the only way to see similar type errors is to > somehow automate the addition of the strict_types declaration to all files > in > your project. While it can be done with tools like sed and awk, > realistically it > means that existing libraries cannot and/or will not benefit easily from > the > changes until they support PHP 7 as a minimum version. Additionally, it > poses > potential problems in terms of different testing results based on which > mode you > run in, leading to ambiguity for users. The only way to address the latter > as a > library author is to specify which mode your code is known to run in. > > Ironically, I'm seeing STHcoerce as more strict than STHv5 in terms of type > handling, due to the fact that every call, userland or internal, is > affected. > The fact that it's always on makes it simpler for me to test against it > now, and > makes it easier to make my existing code both forward-compatible and more > correct — even if I do not add type hints until I update my minimum > supported > version to PHP 7. > > I am slightly worried about the amount of work needed to make code run > from v5 > to v7 if STHcoerce is adopted. That said, the issues I found in PHPUnit > were > corrected in minutes (though a more thorough patch that allows early > return from > methods if doc comments are not strings would be better), and my own > library's > tests ran without problems once the PHPUnit issues were corrected (both > code > with and without typehints). Strict_types is definitely interesting, but I > found > that strict_types mode was more or less how I wanted code to operate, and > to > opt-in to that means changes to every file in my library — which is a much > larger migration problem. > > In the end: kudos to everyone who is working on these patches and RFCs. I'm > excited about scalar type hints in PHP 7! > > > -- > Matthew Weier O'Phinney > Principal Engineer > Project Lead, Zend Framework and Apigility > matthew@zend.com > http://framework.zend.com > http://apigility.org > PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc --- ## #83943 — Pierre Joye — 2015-02-27T00:51:08+00:00 URL: https://externals.io/message/83940#83943 · In reply to: #83940 On Thu, Feb 26, 2015 at 4:29 PM, Matthew Weier O'Phinney wrote: > I've taken some time the last couple days to compile both the Scalare Type Hints > v0.5 (heretofor STHv5) and Coercive Scalar Type Hints (heretofore STHcoerce) > patches and test some code against them. Thanks for this detailed report, much appreciated! > In each case, I modified the `Phly\Http\Response` class from my phly/http > package to add scalar type hints, remove previous argument validation, and then > ran the unit tests. Here's my details of the experience, and analysis. > > ### STHv5 > > With STHv5, my tests failed out of the box. First, I needed to add an error > handler that would convert the `E_RECOVERABLE_ERROR` to an > InvalidArgumentException so that I would get useful error messages from PHPUnit. > Next, once I had done that, I had tests that were throwing invalid input at > methods, and validating that the invalid arguments were caught. This worked in > all but one specific case: passing a float value to an argument expecting an > integer. In this particular case, the value was coerced to an integer, albeit > lossily, and no error was raised. > > When I changed my test case to operate under strict_types mode, the test > executed as I had originally expected, and succeeded. > > #### Analysis > > I did not expect the float value to be coerced, particularly as it had a > fractional part. Yes, I understand that this is how casting _currently_ works in > PHP, and that the patch uses the current casting rules. However, I'd expect that > any float with a fractional value would not be cast to an integer; doing so is > lossy, and can lead to unexpected results. > > The strict_types mode operated how I would expect it, but meant I was forced to > add the declaration to the top of the file. Which leads to this: > > My tests operated differently in strict vs normal mode. That means my code does, > too. Testing both modes would be difficult to do in an automated fashion; > essentially, you're left needing to choose to support one mode or the other. > This goes back to the point I was making earlier this week: I worry that having > two modes will lead to a schism in userland libraries: those that support > strict, and those that do not; there will be little possibility of testing both. It seems you still are still confused about that one :) This opt-in mode is the key value of this RFC. Nothing changes if you do not care and do not want to care. And if you care and want to port part of your code, you can, being fully informed about what it means for this file. > ### STHcoerce > > With STHcoerce, my tests also failed out of the box, but not for the same > reason. Instead, I had a bunch of errors reported based on code that PHPUnit was > executing! In this case, I discovered that: > > - PHPUnit passes a boolean false to `debug_backtrace()`... which is documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is relying > on the fact that the engine casts booleans to the integers 0 and 1. (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) And now let wait all other cases not covered by errors but casting to different values, maybe only a few, maybe none, we simply do not know. Cheers, --- ## #83953 — francois@php.net — 2015-02-27T02:06:06+00:00 URL: https://externals.io/message/83940#83953 · In reply to: #83940 > De : Matthew Weier O'Phinney [mailto:matthew@zend.com] > > - PHPUnit passes a boolean false to `debug_backtrace()`... which is > documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is relying > on the fact that the engine casts booleans to the integers 0 and 1. (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) AFAIK, we won't support boolean to integer. IMO, considering Boolean as integer is a bug and must not be supported. > - PHPUnit is passing the results of $reflector->getDocComment() blindly to > `substr()` and preg_match*(). getDocComment() is documented as returning > EITHER > a string OR boolean false. Again, PHPUnit is relying on PHP to cast boolean > false to an empty string. (Zeev has also indicated this coercion path may be > re-introduced.) The same as above for bool -> string. I hope you're wrong because I wouldn't like supporting boolean to scalar back again. Your test demonstrates this because you found undetected bugs. I am more and more sure that, what I first said as a joke, will prove true : during the next years, STH will be used mostly as a debugging tool, proving opposite arguments were FUD or, at least, phantasm. Regards François --- ## #83956 — francois@php.net — 2015-02-27T02:48:14+00:00 URL: https://externals.io/message/83940#83956 · In reply to: #83943 > De : Pierre Joye [mailto:pierre.php@gmail.com] > And now let wait all other cases not covered by errors but casting to > different values, maybe only a few, maybe none, we simply do not know. Pierre, excuse me to repeat here, because I jst replied the same in another thread but there is no 'casting to different values'. That's why I didn't understand when you adked about failures on different casting. This is a pure subset. We diable cases, but absolutely don't change conversions. I agree that changing conversion rules would be terrible. It is absolutely not the case. Our only fault is to have made this not clear enough in the RFC. Regards François --- ## #83969 — Xinchen Hui — 2015-02-27T07:59:09+00:00 URL: https://externals.io/message/83940#83969 · In reply to: #83953 Hey: > > […] > > AFAIK, we won't support boolean to integer. IMO, considering Boolean as integer is a bug and must not be supported. in a internal developer's view: bool false === 0, bool true == 1; maybe this case should be supported? considering such usage : if (substr($str, strrpos($str, ':delimiter:')) { } this is not a really world usage, I just made it up, this codes is try to find a last part of a string which is contacted with a dimileter ":delimiter": like a:delimiter:b, c:dilimiter:d it works well before, because, if there is no match, it returns false before.. (which we will think it's '0') then the whole $str is returned.. but if we don't support bool false -> 0? thanks > > > […] > > The same as above for bool -> string. > > I hope you're wrong because I wouldn't like supporting boolean to scalar back again. > > Your test demonstrates this because you found undetected bugs. I am more and more sure that, what I first said as a joke, will prove true : during the next years, STH will be used mostly as a debugging tool, proving opposite arguments were FUD or, at least, phantasm. > > Regards > > François > > > > -- > > -- Xinchen Hui @Laruence http://www.laruence.com/ --- ## #83970 — Xinchen Hui — 2015-02-27T08:01:20+00:00 URL: https://externals.io/message/83940#83970 · In reply to: #83969 Hey: > Hey: > > > > […] > in a internal developer's view: > > bool false === 0, bool true == 1; > > maybe this case should be supported? > > considering such usage : > > if (substr($str, strrpos($str, ':delimiter:')) { > > } > > this is not a really world usage, I just made it up, this codes is try > to find a last part of a string which is contacted with a dimileter > ":delimiter": sorry, the example is wrong, but I think you may get my idea. a works example is: find the first part: substr($str, 0, strpos($str, ":delimiter:")) which is common used,, right? thanks > > like a:delimiter:b, c:dilimiter:d > > it works well before, because, if there is no match, it returns false > before.. (which we will think it's '0') > > then the whole $str is returned.. > > but if we don't support bool false -> 0? > > thanks > > […] > > > > -- > Xinchen Hui > @Laruence > http://www.laruence.com/ -- Xinchen Hui @Laruence http://www.laruence.com/ --- ## #83973 — Benjamin Eberlei — 2015-02-27T08:08:53+00:00 URL: https://externals.io/message/83940#83973 · In reply to: #83940 Matthew, On Fri, Feb 27, 2015 at 1:29 AM, Matthew Weier O'Phinney wrote: > I've taken some time the last couple days to compile both the Scalare Type > Hints > v0.5 (heretofor STHv5) and Coercive Scalar Type Hints (heretofore > STHcoerce) > patches and test some code against them. > > In each case, I modified the `Phly\Http\Response` class from my phly/http > package to add scalar type hints, remove previous argument validation, and > then > ran the unit tests. Here's my details of the experience, and analysis. > > ### STHv5 > > With STHv5, my tests failed out of the box. First, I needed to add an error > handler that would convert the `E_RECOVERABLE_ERROR` to an > InvalidArgumentException so that I would get useful error messages from > PHPUnit. > Next, once I had done that, I had tests that were throwing invalid input at > methods, and validating that the invalid arguments were caught. This > worked in > all but one specific case: passing a float value to an argument expecting > an > integer. In this particular case, the value was coerced to an integer, > albeit > lossily, and no error was raised. > > When I changed my test case to operate under strict_types mode, the test > executed as I had originally expected, and succeeded. > > #### Analysis > > I did not expect the float value to be coerced, particularly as it had a > fractional part. Yes, I understand that this is how casting _currently_ > works in > PHP, and that the patch uses the current casting rules. However, I'd > expect that > any float with a fractional value would not be cast to an integer; doing > so is > lossy, and can lead to unexpected results. > > The strict_types mode operated how I would expect it, but meant I was > forced to > add the declaration to the top of the file. Which leads to this: > > My tests operated differently in strict vs normal mode. That means my code > does, > too. Testing both modes would be difficult to do in an automated fashion; > essentially, you're left needing to choose to support one mode or the > other. > This goes back to the point I was making earlier this week: I worry that > having > two modes will lead to a schism in userland libraries: those that support > strict, and those that do not; there will be little possibility of testing > both. > > ### STHcoerce > > With STHcoerce, my tests also failed out of the box, but not for the same > reason. Instead, I had a bunch of errors reported based on code that > PHPUnit was > executing! In this case, I discovered that: > > - PHPUnit passes a boolean false to `debug_backtrace()`... which is > documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is > relying > on the fact that the engine casts booleans to the integers 0 and 1. > (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) > - PHPUnit is passing the results of $reflector->getDocComment() blindly to > `substr()` and preg_match*(). getDocComment() is documented as returning > EITHER > a string OR boolean false. Again, PHPUnit is relying on PHP to cast > boolean > false to an empty string. (Zeev has also indicated this coercion path > may be > re-introduced.) > > I was able to fix these in a matter of minutes, and then my tests ran, and > passed without any changes. > > #### Analysis > > STHcoerce worked about how I expect STHv5 will work _if_ _every_ _file_ > were > declared in strict_types mode. In other words, it uncovered errors in > calling > both userland and internal functions. Userland typehints worked as I > expected, > with floats using fractional values not casting to integers. > > ### Wrap Up > > STHcoerce was actually far more strict than I found strict_types mode to > be in > STHv5. The reason is that it's a single, non-optional mode. This poses a > potential challenge to migration, as noted in my problems using PHPUnit > (Yes, I > WILL be sending patches their way soon!): method calls and arguments that > previously worked because of how PHP juggles types often do not work, > particularly when going from boolean to other scalar values. However, the > patch > does what I'd expect in terms of preventing operations that would result in > either data loss or data additions, all of which can have unexpected side > effects if you don't understand the casting rules currently. > > The flip side to this is that you do not need to make any changes to your > code > in order to locate and fix errors. What this means from a library/framework > author perspective is that, if the STHcoerce patch is merged, I can test > against > PHP 7, make fixes, and my code is not only forward-compatible, but also > backwards to the various PHP 5 versions I might already be supporting — > and that > code now benefits from being more correct. > > Because STHv5 is opt-in only, the only way to see similar type errors is to > somehow automate the addition of the strict_types declaration to all files > in > your project. While it can be done with tools like sed and awk, > realistically it > means that existing libraries cannot and/or will not benefit easily from > the > changes until they support PHP 7 as a minimum version. Additionally, it > poses > potential problems in terms of different testing results based on which > mode you > run in, leading to ambiguity for users. The only way to address the latter > as a > library author is to specify which mode your code is known to run in. > > Ironically, I'm seeing STHcoerce as more strict than STHv5 in terms of type > handling, due to the fact that every call, userland or internal, is > affected. > The fact that it's always on makes it simpler for me to test against it > now, and > makes it easier to make my existing code both forward-compatible and more > correct — even if I do not add type hints until I update my minimum > supported > version to PHP 7. > > I am slightly worried about the amount of work needed to make code run > from v5 > to v7 if STHcoerce is adopted. That said, the issues I found in PHPUnit > were > corrected in minutes (though a more thorough patch that allows early > return from > methods if doc comments are not strings would be better), and my own > library's > tests ran without problems once the PHPUnit issues were corrected (both > code > with and without typehints). Strict_types is definitely interesting, but I > found > that strict_types mode was more or less how I wanted code to operate, and > to > opt-in to that means changes to every file in my library — which is a much > larger migration problem. > > In the end: kudos to everyone who is working on these patches and RFCs. I'm > excited about scalar type hints in PHP 7! > Can you gist the PHPUnit changes and point us to the coercive patch you compiled? Your results seem to be: 1. Weak mode in Dual Mode patch is too weak. 2. Coercive patch gives good feedback without strict. 3. Strict is too strict to apply to *all* your code retroactively that hasn't been designed for this from the beginning. Ironically if both RFCs are accepted you would get the best of your world, because Coercive STH is only a tightening of the rules for Weak mode of STHDual. > > -- > Matthew Weier O'Phinney > Principal Engineer > Project Lead, Zend Framework and Apigility > matthew@zend.com > http://framework.zend.com > http://apigility.org > PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc --- ## #83989 — Zeev Suraski — 2015-02-27T09:48:22+00:00 URL: https://externals.io/message/83940#83989 · In reply to: #83940 > - PHPUnit passes a boolean false to `debug_backtrace()`... which is > documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is > relying > on the fact that the engine casts booleans to the integers 0 and 1. > (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) Matthew, I just wanted to point out that actually, supporting boolean->int coercion is not planned. Converting boolean to integer is actually a very likely scenario where you'd have an outcome that you didn't expect. It's debatable whether sending false to a bitmask is OK or not, but given it's a bitmask, an explicit 0 is a lot more correct. > - PHPUnit is passing the results of $reflector->getDocComment() blindly to > `substr()` and preg_match*(). getDocComment() is documented as returning > EITHER > a string OR boolean false. Again, PHPUnit is relying on PHP to cast > boolean > false to an empty string. (Zeev has also indicated this coercion path > may be > re-introduced.) We're looking to re-introduce string->boolean (which will likely be common in database scenarios), but not the other way around - where it's very likely hiding a bug. Virtually all of the boolean->string coercions that were flagged as deprecated by the patch were at the very least suspects of being real world bugs. Thanks! Zeev --- ## #84001 — Jordi Boggiano — 2015-02-27T13:44:49+00:00 URL: https://externals.io/message/83940#84001 · In reply to: #83940 Matthew, Thanks a bunch for going through this and writing such a detailed report. > ### STHv5 > [snip] > #### Analysis > > I did not expect the float value to be coerced, particularly as it had a > fractional part. Yes, I understand that this is how casting _currently_ works in > PHP, and that the patch uses the current casting rules. However, I'd expect that > any float with a fractional value would not be cast to an integer; doing so is > lossy, and can lead to unexpected results. > > The strict_types mode operated how I would expect it, but meant I was forced to > add the declaration to the top of the file. Which leads to this: > > My tests operated differently in strict vs normal mode. That means my code does, > too. Testing both modes would be difficult to do in an automated fashion; > essentially, you're left needing to choose to support one mode or the other. > This goes back to the point I was making earlier this week: I worry that having > two modes will lead to a schism in userland libraries: those that support > strict, and those that do not; there will be little possibility of testing both. I think this last bit is misguided. Your *tests* are running differently depending if they are in strict and weak mode, that is correct, but the code's behavior itself is not changing based on the test suite's strictness. That does not mean your library is tested to run in mode X or Y, it just means your test suite's correctness relies on testing the PHP type hints in this case, and it breaks when you change the way the hints behave. For users the code will work the same no matter if they are in strict or weak mode, but if they are in weak mode they should indeed watch out if they send floats they will be silently truncated as those are the rules of weak mode. > In the end: kudos to everyone who is working on these patches and RFCs. I'm > excited about scalar type hints in PHP 7! I agree with Benjamin's conclusion that having both RFCs combined might be for the best. At the very least lossy float->int should be disabled in the STHv5's weak mode because that's dangerous and unlikely to occur IMO. The other casting changes from STHcoerce I am not sure if they're for the best or not. I am a bit worried we change things arbitrarily based on who has a good argument not to change them, and we might end up with a completely inconsistent set of casting rules. Cheers --- ## #84015 — Sebastian Bergmann — 2015-02-27T15:25:42+00:00 URL: https://externals.io/message/83940#84015 · In reply to: #83940 Am 27.02.2015 um 01:29 schrieb Matthew Weier O'Phinney: > - PHPUnit passes a boolean false to `debug_backtrace()`... which is documented > as expecting an integer! (There are actually several constant values it > accepts, all of which are integer values.) In this case, PHPUnit is relying > on the fact that the engine casts booleans to the integers 0 and 1. (Zeev has > written to the list already indicating that this coercion path will be > supported in the patch.) > - PHPUnit is passing the results of $reflector->getDocComment() blindly to > `substr()` and preg_match*(). getDocComment() is documented as returning EITHER > a string OR boolean false. Again, PHPUnit is relying on PHP to cast boolean > false to an empty string. (Zeev has also indicated this coercion path may be > re-introduced.) Pull requests for PHPUnit would be appreciated ;-) --- ## #84020 — Benjamin Eberlei — 2015-02-27T15:31:29+00:00 URL: https://externals.io/message/83940#84020 · In reply to: #84015 On Fri, Feb 27, 2015 at 4:25 PM, Sebastian Bergmann wrote: > Am 27.02.2015 um 01:29 schrieb Matthew Weier O'Phinney: > > […] > documented > > […] > relying > > […] > (Zeev has > > […] > to > > […] > EITHER > > […] > boolean > > […] > may be > > […] > > Pull requests for PHPUnit would be appreciated ;-) https://gist.github.com/beberlei/8a33ae940829f1186da2 <- these are the necessary changes for dev-msater to run on php7+coercive patch