# RFC: expectations/assertions - Thread: https://externals.io/message/71992 - Original archive: https://news-web.php.net/php.internals/71992 - Started by Joe Watkins on 2014-02-02T11:25:17+00:00 - Messages: 19 - 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 --- ## #71992 — Joe Watkins — 2014-02-02T11:25:17+00:00 URL: https://externals.io/message/71992#71992 Morning Internals [, and Dmitry :)], I came across a reason to think about assertions again today, my original, pretty radical, patch was worked on by dmitry, I updated the RFC a while ago but allowed the conversation to die down. Nikita pointed this out around the same time: https://gist.github.com/krakjoe/4a2145e5a6ddc26b1dc1 Still, there's no reason for us to have a crappy assertion api as part of the core just because there's a way around it(-ish) in user land. So, I think now is a good time to revive the discussion, the patch is pretty unobjectionable, and has been updated for master. I'd like to move this to a vote pretty swiftly, any objections ? Cheers Joe --- ## #71994 — Joe Watkins — 2014-02-02T11:50:54+00:00 URL: https://externals.io/message/71992#71994 · In reply to: #71992 > Morning Internals [, and Dmitry :)], > > I came across a reason to think about assertions again today, my > original, pretty radical, patch was worked on by dmitry, I updated the > RFC a while ago but allowed the conversation to die down. > > Nikita pointed this out around the same time: > > https://gist.github.com/krakjoe/4a2145e5a6ddc26b1dc1 > > Still, there's no reason for us to have a crappy assertion api as > part of the core just because there's a way around it(-ish) in user land. > > So, I think now is a good time to revive the discussion, the patch > is pretty unobjectionable, and has been updated for master. > > I'd like to move this to a vote pretty swiftly, any objections ? > > Cheers > Joe Woops2, https://gist.github.com/krakjoe/c6bcbb4e6d019c9297db Dunno what happened there .. Cheers Joe --- ## #72014 — Stas Malyshev — 2014-02-02T20:17:02+00:00 URL: https://externals.io/message/71992#72014 · In reply to: #71992 Hi! > Still, there's no reason for us to have a crappy assertion api as part > of the core just because there's a way around it(-ish) in user land. I think there is some reason - if it can easily be done in userland, with more flexibility and suitability for particular purposes of the user, why put it in core? If it's such a common feature, make a PSR for it and create reference implementation that everybody would be using. So my question is - what having in core adds to it that can't be done as easily in userland? Also, it essentially splits `assert()` into two completely independent things one of them being regular function that you can call and another being weird magic thing. Moreover, for the same assert there's now two completely independent ways of controlling it - one is zend.assert and another is assert_options/ASSERT_ACTIVE. E.g. compare: // This is controlled by zend.assert assert($foo > 1, "Foo is too small"); // This is controlled by assert_options $f = 'assert'; $f($foo > 1, "Foo is too small"); This looks a bit strange for me. Also, `assert()` will throw exception if assert.bail = 1 and assert.exception = 1, but only if the argument is not a string. If the argument is a string no exception will be thrown. OTOH, if the argument IS a string, assert will both throw exception *and* bail out, which is not clear what is the purpose of that. I'm also feeling a bit uneasy with adding more and more special cases for special names in the engine. Maybe we could create some more generic facility that would not need to be one-off case for just one function? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #72031 — Joe Watkins — 2014-02-03T01:51:50+00:00 URL: https://externals.io/message/71992#72031 · In reply to: #72014 > Hi! > > > […] > I think there is some reason - if it can easily be done in userland, > with more flexibility and suitability for particular purposes of the > user, why put it in core? If it's such a common feature, make a PSR for > it and create reference implementation that everybody would be using. So > my question is - what having in core adds to it that can't be done as > easily in userland? > > Also, it essentially splits `assert()` into two completely independent > things one of them being regular function that you can call and another > being weird magic thing. Moreover, for the same assert there's now two > completely independent ways of controlling it - one is zend.assert and > another is assert_options/ASSERT_ACTIVE. E.g. compare: > > // This is controlled by zend.assert > assert($foo > 1, "Foo is too small"); > > // This is controlled by assert_options > $f = 'assert'; > $f($foo > 1, "Foo is too small"); > > This looks a bit strange for me. > > Also, `assert()` will throw exception if assert.bail = 1 and > assert.exception = 1, but only if the argument is not a string. If the > argument is a string no exception will be thrown. OTOH, if the argument > IS a string, assert will both throw exception *and* bail out, which is > not clear what is the purpose of that. > > I'm also feeling a bit uneasy with adding more and more special cases > for special names in the engine. Maybe we could create some more generic > facility that would not need to be one-off case for just one function? Well lets not forget that we already have an implementation as part of the core, so to argue that we don't require assert in the core is moot; we already have it. While you can avoid some overhead in userland, you cannot remove the overhead completely; a good assertion API should have no impact on production, none, it should require no boilerplate code to make sane use of it, at all, since it is meant to be a core feature. It would seem to create inconsistency, but only because it's compatible with legacy code asserting strings, if you are smart, you will not assert with strings and take advantage of the new implementation, if you are not, you will continue to assert strings and be unaffected by the new implementation ... that seems ideal to me, and Dmitry ... Cheers Joe --- ## #72058 — Stas Malyshev — 2014-02-03T08:38:32+00:00 URL: https://externals.io/message/71992#72058 · In reply to: #72031 Hi! > Well lets not forget that we already have an implementation as part of > the core, so to argue that we don't require assert in the core is moot; > we already have it. I'm not sure what you mean. We have assert function, a function among others. What you propose is special engine-level magic just for this function. The difference is like having a program that runs on Linux and does X and having special patch in Linux kernel just to do X. The bar for the latter is much higher. > While you can avoid some overhead in userland, you cannot remove the > overhead completely; a good assertion API should have no impact on > production, none, it should require no boilerplate code to make sane use > of it, at all, since it is meant to be a core feature. This is not completely fulfilled by your patch either - it still has an overhead of having the opcodes there and doing the jumps. If the code you're avoiding to run is heavy, then of course this overhead is negligible compared to the code you're avoiding, but the same then is true for purely userspace solution. > It would seem to create inconsistency, but only because it's compatible > with legacy code asserting strings, if you are smart, you will not It doesn't "seem" to create inconsistency. It creates it. It's not like I'm imagining two parameters doing the same thing :) > assert with strings and take advantage of the new implementation, if you > are not, you will continue to assert strings and be unaffected by the > new implementation ... that seems ideal to me, and Dmitry ... I don't think having a function that is half a function and half a magic engine constraint and that is controlled by two separate settings, one living in zend. and one in assert. is really ideal. Also, it is not true that string parameter is the only thing that controls it - as I already showed, how you call it also makes it work completely different and assert.* parameters still influence it even in magic mode, leading to such things as both throwing exceptions and bailing out - which I still don't understand how it's supposed to work. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #72059 — Joe Watkins — 2014-02-03T09:09:53+00:00 URL: https://externals.io/message/71992#72059 · In reply to: #72058 > Hi! > > > […] > I'm not sure what you mean. We have assert function, a function among > others. What you propose is special engine-level magic just for this > function. The difference is like having a program that runs on Linux and > does X and having special patch in Linux kernel just to do X. The bar > for the latter is much higher. > > > […] > This is not completely fulfilled by your patch either - it still has an > overhead of having the opcodes there and doing the jumps. If the code > you're avoiding to run is heavy, then of course this overhead is > negligible compared to the code you're avoiding, but the same then is > true for purely userspace solution. > > > […] > It doesn't "seem" to create inconsistency. It creates it. It's not like > I'm imagining two parameters doing the same thing :) > > > […] > I don't think having a function that is half a function and half a magic > engine constraint and that is controlled by two separate settings, one > living in zend. and one in assert. is really ideal. Also, it is not true > that string parameter is the only thing that controls it - as I already > showed, how you call it also makes it work completely different and > assert.* parameters still influence it even in magic mode, leading to > such things as both throwing exceptions and bailing out - which I still > don't understand how it's supposed to work. > > This is not completely fulfilled by your patch either - it still has an > overhead of having the opcodes there and doing the jumps. If the code > you're avoiding to run is heavy, then of course this overhead is > negligible compared to the code you're avoiding, but the same then is > true for purely userspace solution. As it says in the RFC, the complete removal will be implemented before merge, the implementation allows the complete removal, the one in the core doesn't. The original patch didn't have this limitation but wasn't compatible with the existing assert API, so dmitry decided this way was better. > It doesn't "seem" to create inconsistency. It creates it. It's not like > I'm imagining two parameters doing the same thing :) It doesn't create inconsistency in code currently using assert, and there's no such thing as inconsistent code that doesn't exist, so I don't see the inconsistency you think is definitely there, I see how you think it's there, without really considering it, but it is not in fact there. Code currently using assert will behave in the same way, code taking advantage of exceptions and expressions as arguments will behave in the way the patch dictates. I see no inconsistency. > I don't think having a function that is half a function and half a magic > engine constraint and that is controlled by two separate settings, one > living in zend. and one in assert. is really ideal. Also, it is not true > that string parameter is the only thing that controls it - as I already > showed, how you call it also makes it work completely different and > assert.* parameters still influence it even in magic mode, leading to > such things as both throwing exceptions and bailing out - which I still > don't understand how it's supposed to work. Nor do I, the original patch wasn't this, it was a new syntax with this API that solved the problems this patch is solving in the same way but by superseding it's functionality rather than trying to retain compatibility with something that is crappy. Since that turned out to not be an option - we have to retain compatibility with the current assert api - the current patch is the only way of moving forward, even if you can see ways of making it do strange things (as a result of retaining compatibility), the perfectly clean patch, with no strangeness whatsoever, was rejected because we don't like to introduce new keywords or syntax or blah blah blah ... If you use the configuration you had before it will behave as it did before, if you setup the new configuration and use the new api it will behave as the patch describes, that you can purposefully configure or otherwise force php to do strange things is nothing new. I'm going to move it to vote today, there's not much point in talking about it, the discussions have already been had and the patch rewritten to satisfy the complaints of most people, and has also been available for many many months, so if there is a suitable patch, this is it, I don't want to waste more time on it if we're not going to move forward. Cheers Joe --- ## #72060 — Pierre Joye — 2014-02-03T09:11:48+00:00 URL: https://externals.io/message/71992#72060 · In reply to: #72058 > Hi! > > > […] > > I'm not sure what you mean. We have assert function, a function among > others. What you propose is special engine-level magic just for this > function. The difference is like having a program that runs on Linux and > does X and having special patch in Linux kernel just to do X. The bar > for the latter is much higher. > > > […] > > This is not completely fulfilled by your patch either - it still has an > overhead of having the opcodes there and doing the jumps. If the code > you're avoiding to run is heavy, then of course this overhead is > negligible compared to the code you're avoiding, but the same then is > true for purely userspace solution. > > > […] > > It doesn't "seem" to create inconsistency. It creates it. It's not like > I'm imagining two parameters doing the same thing :) > > > […] > > I don't think having a function that is half a function and half a magic > engine constraint and that is controlled by two separate settings, one > living in zend. and one in assert. is really ideal. Also, it is not true > that string parameter is the only thing that controls it - as I already > showed, how you call it also makes it work completely different and > assert.* parameters still influence it even in magic mode, leading to > such things as both throwing exceptions and bailing out - which I still > don't understand how it's supposed to work. I fully agree here. As the effort to reduce the impact of assert in production is much appreciated I do not see much needs for this "addition". I also discussed with a couple of developers being intensively involved in testing tools and framework for php and the consensus is that they do not need it for the large majority. A minority even considers this feature as not really good, assert never was a good thing to use (I never used for example). Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #72065 — Yasuo Ohgaki — 2014-02-03T09:59:55+00:00 URL: https://externals.io/message/71992#72065 · In reply to: #72060 Hi Pierre, > I also discussed with a couple of developers being intensively > involved in testing tools and framework for php and the consensus is > that they do not need it for the large majority. A minority even > considers this feature as not really good, assert never was a good > thing to use (I never used for example). > For testers and test tools developers, or even framework developer, assertion may not be important. Framework may have contract via annotations. However, it is important for developers who is not rely on framework to make sure data is processed as developer expects. It would be important for portable library developers especially. Since they do not want to rely on specific frameworks nor they cannot assume user tests/code are valid one. Assertion works perfectly for portable library developers to achieve both performance and reliability. Assertion is useful for basic form of DbC. DbC could achieve fast, yet reliable applications. Extensive use of assertion is required if developer choose to do so. It is important to remove assertion overheads from production code. Therefore, this RFC is valuable people who care about DbC and prefer to use light weight frameworks like myself. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #72069 — Lester Caine — 2014-02-03T10:19:58+00:00 URL: https://externals.io/message/71992#72069 · In reply to: #72065 Yasuo Ohgaki wrote: > It is important to remove assertion overheads from production code. > Therefore, this RFC is valuable people who care about DbC and prefer to use > light weight frameworks like myself. And when did we agree to support DbC in the core? It really is feeling as if there are two versions of PHP being developed ... the one everybody uses and another 'fancy version' with all sorts of bells and whistles and just a very small minority want. Why can't all of these extensions simply be supported by third party tools ... which are already available ... and keep the core as simple as possible. -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #72071 — Derick Rethans — 2014-02-03T10:20:50+00:00 URL: https://externals.io/message/71992#72071 · In reply to: #72031 > While you can avoid some overhead in userland, you cannot remove the > overhead completely; a good assertion API should have no impact on > production, none, it should require no boilerplate code to make sane > use of it, at all, since it is meant to be a core feature. I think Stas' main objection was the inconsistencies that the "new assert" method introduces. And I have to agree with that there. It shouldn't depend on whether you use a string or not how the construct/function changes code execution and exception throwing. cheers, Derick -- http://derickrethans.nl | http://xdebug.org Like Xdebug? Consider a donation: http://xdebug.org/donate.php twitter: @derickr and @xdebug Posted with an email client that doesn't mangle email: alpine --- ## #72089 — Levi Morrison — 2014-02-03T17:18:01+00:00 URL: https://externals.io/message/71992#72089 · In reply to: #72060 I support the idea behind improving assert; I really should be able to say 'disable assertions' somehow and then assertions would have no impact at runtime. I have not reviewed the changes for the revised RFC at this time, but I wanted to voice my support for the general idea. Also: > assert never was a good thing to use This is just nonsense. Properly used asserts are fundamental in defensive programming and you can quite a bit of literature about it. Perhaps Pierre meant to say, "assert never was a good thing to use *in PHP*" and then I would agree with him because of the currently poor implementation, not because they are bad in principle. --- ## #72090 — Pierre Joye — 2014-02-03T17:28:24+00:00 URL: https://externals.io/message/71992#72090 · In reply to: #72089 > > I support the idea behind improving assert; I really should be able to say 'disable assertions' somehow and then assertions would have no impact at runtime. I have not reviewed the changes for the revised RFC at this time, but I wanted to voice my support for the general idea. > > Also: > > > […] > > This is just nonsense. Properly used asserts are fundamental in defensive programming and you can quite a bit of literature about it. Perhaps Pierre meant to say, "assert never was a good thing to use *in PHP*" Indeed in php, and I do not use it in c either btw. --- ## #72091 — Dmitry Stogov — 2014-02-03T17:29:05+00:00 URL: https://externals.io/message/71992#72091 · In reply to: #72058 Hi Stas, In main opinion, the main advantage of the proposal is ability to use of `assert()` function in production code with zero cost. It might be important, because cost of the `assert()` call and condition evaluation might be expensive. It's similar to C which provides `assert()` function, but near every project defines ASSERT macro anyway, to eliminate `assert()` calls for release build. Thanks. Dmitry. On Mon, Feb 3, 2014 at 12:38 PM, Stas Malyshev wrote: > Hi! > > > […] > > I'm not sure what you mean. We have assert function, a function among > others. What you propose is special engine-level magic just for this > function. The difference is like having a program that runs on Linux and > does X and having special patch in Linux kernel just to do X. The bar > for the latter is much higher. > > > […] > > This is not completely fulfilled by your patch either - it still has an > overhead of having the opcodes there and doing the jumps. If the code > you're avoiding to run is heavy, then of course this overhead is > negligible compared to the code you're avoiding, but the same then is > true for purely userspace solution. > > > […] > > It doesn't "seem" to create inconsistency. It creates it. It's not like > I'm imagining two parameters doing the same thing :) > > > […] > > I don't think having a function that is half a function and half a magic > engine constraint and that is controlled by two separate settings, one > living in zend. and one in assert. is really ideal. Also, it is not true > that string parameter is the only thing that controls it - as I already > showed, how you call it also makes it work completely different and > assert.* parameters still influence it even in magic mode, leading to > such things as both throwing exceptions and bailing out - which I still > don't understand how it's supposed to work. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #72124 — Yasuo Ohgaki — 2014-02-03T21:14:37+00:00 URL: https://externals.io/message/71992#72124 · In reply to: #72069 Hi Lester, > Yasuo Ohgaki wrote: > > > […] > > And when did we agree to support DbC in the core? > It really is feeling as if there are two versions of PHP being developed > ... the one everybody uses and another 'fancy version' with all sorts of > bells and whistles and just a very small minority want. > Why can't all of these extensions simply be supported by third party tools > ... which are already available ... and keep the core as simple as possible. `assert()` is not new feature, but this RFC is for improving it. Current `assert()` will call `NULL` function at run time. Therefore, there would be overheads that cannot be ignored when `assert()` is used extensively. With this RFC, run time overheads are removed. I agree that this RFC would change the way script is coded. Assertion can be used for DbC, but it's up to users if they use it for DbC or not. No existing code is affected in any way. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #72136 — Stas Malyshev — 2014-02-03T21:58:33+00:00 URL: https://externals.io/message/71992#72136 · In reply to: #72091 Hi! > In main opinion, the main advantage of the proposal is ability to use of > `assert()` function in production code with zero cost. > It might be important, because cost of the `assert()` call and condition > evaluation might be expensive. I agree, this would be nice. Though it can be implemented using closures and calling function only when needed. Granted, it is less convenient and I'd like to have generic conditional compilation facility that could help with it (or compiler/optimizer smart enough that if() could be eliminated by it) but doing it as one-off hack for one specific function doesn't look like ideal solution to me. BTW, current patch has zend.assertions as INI_SYSTEM, which means the whole server can either have assertions on or off. Which means if you want to have two sites with different settings (like staging and production, or production and debug copy), you'd have to set up separate PHP server for each. > It's similar to C which provides `assert()` function, but near every > project defines ASSERT macro anyway, to eliminate `assert()` calls for > release build. In C, you can do it with NDEBUG setting, which works a bit differently - you compile it once, and then it's predictable forever. With PHP it's not that easy, unfortunately. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #72170 — Dmitry Stogov — 2014-02-04T05:36:27+00:00 URL: https://externals.io/message/71992#72170 · In reply to: #72136 On Tue, Feb 4, 2014 at 1:58 AM, Stas Malyshev wrote: > Hi! > > > […] > > I agree, this would be nice. Though it can be implemented using closures > and calling function only when needed. Granted, it is less convenient > and I'd like to have generic conditional compilation facility that could > help with it (or compiler/optimizer smart enough that if( false>) could be eliminated by it) Unfortunately, optimizer is not smart enough to solve this problem (on purpose). It may substitute only internal constants and constants defined in the same file > but doing it as one-off hack for one > specific function doesn't look like ideal solution to me. > I wouldn't say it's an ideal solution. It's more a compromise that allows safer checks in debugging and running in production without performance lose. The solution also keeps full BC with old `assert()` behavior, that wasn't trivial :) > > BTW, current patch has zend.assertions as INI_SYSTEM, which means the > whole server can either have assertions on or off. It's `INI_SYSTEM` on purpose. zend.assertions is a 3 way switch that may disable code generation for `assert()`. Changing it after PHP files are already cached won't make any effect. > Which means if you > want to have two sites with different settings (like staging and > production, or production and debug copy), you'd have to set up separate > PHP server for each. > yes. > > > > […] > > In C, you can do it with NDEBUG setting, which works a bit differently - > you compile it once, and then it's predictable forever. With PHP it's > not that easy, unfortunately. > So, C provides special handling for `assert()` as well :) Thanks. Dmitry. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #72209 — Rowan Collins — 2014-02-04T20:54:38+00:00 URL: https://externals.io/message/71992#72209 · In reply to: #71992 > Morning Internals [, and Dmitry :)], > > I came across a reason to think about assertions again today, my > original, pretty radical, patch was worked on by dmitry, I updated the > RFC a while ago but allowed the conversation to die down. > > Nikita pointed this out around the same time: > > https://gist.github.com/krakjoe/4a2145e5a6ddc26b1dc1 > > Still, there's no reason for us to have a crappy assertion api as > part of the core just because there's a way around it(-ish) in user land. > > So, I think now is a good time to revive the discussion, the patch > is pretty unobjectionable, and has been updated for master. > > I'd like to move this to a vote pretty swiftly, any objections ? > > Cheers > Joe > This is just a comment from someone without massively strong opinions on the proposal, but to me the RFC doesn't really make clear which parts of the proposed version of `assert()` are actually new. For instance, all the examples in the "Scope of Assertions" section are actually valid in current PHP, and if written to use eval()d strings could be used as the manual page for `assert()` today. Without diving into the patch, I understand there to be two, somewhat independent, changes proposed here: 1) An ability to by-pass the asserted expression in the compiler but lay it out as plain PHP code, rather than an eval()'d string, combining the advantages of both versions currently supported. Some of your posts suggest that it is also more efficient even than the eval()d string version, but this isn't really clarified in the RFC. A new ini setting, zend.assert, is added to replace (or supplement?) assert.active to control this part of the behaviour. 2) A mode to throw Exceptions, rather than Warnings or just bailing, when an assertion fails. The first part of this is an extra assert option, assert.exceptions, which will cause the assertion to throw an AssertionException if it fails. In addition, the second parameter ($message) is extended so that if an object descending from AssertException is passed, that will be thrown instead. Passing such an object in is probably only efficient in conjunction with the ability of the compiler to skip the entire expression, so somewhat relies on both of the above features. Does this sound like a fair summary? If so, perhaps it could be added as an introduction in the RFC? Regards, -- Rowan Collins [IMSoP] --- ## #72210 — Rowan Collins — 2014-02-04T20:57:35+00:00 URL: https://externals.io/message/71992#72210 · In reply to: #72170 > > […] > It's `INI_SYSTEM` on purpose. zend.assertions is a 3 way switch that may > disable code generation for `assert()`. > Changing it after PHP files are already cached won't make any effect. > > Incidentally, if this is true, the introduction in the RFC is incorrect, as it currently states: > assertions can therefore be disabled and enabled via the PHP_INI_ALL configuration setting zend.assertions The explanation of *why* it would be `INI_SYSTEM` could also be added there. -- Rowan Collins [IMSoP] --- ## #72242 — Dmitry Stogov — 2014-02-05T06:53:39+00:00 URL: https://externals.io/message/71992#72242 · In reply to: #72210 It's `INI_SYSTEM` because it may affect the compiled code that might be cached. So changing it on second request won't affect already cache code. Thanks. Dmitry. On Wed, Feb 5, 2014 at 12:57 AM, Rowan Collins wrote: > > > > […] > Incidentally, if this is true, the introduction in the RFC is incorrect, > as it currently states: > > > […] > configuration setting zend.assertions > > The explanation of *why* it would be `INI_SYSTEM` could also be added there. > > -- > Rowan Collins > [IMSoP]