Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:69681 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 47653 invoked from network); 18 Oct 2013 13:50:26 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 18 Oct 2013 13:50:26 -0000 X-Host-Fingerprint: 80.4.21.210 cpc22-asfd3-2-0-cust209.1-2.cable.virginmedia.com Received: from [80.4.21.210] ([80.4.21.210:15866] helo=localhost.localdomain) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id DB/A4-23638-1AC31625 for ; Fri, 18 Oct 2013 09:50:25 -0400 To: internals@lists.php.net,Ferenc Kovacs Message-ID: <52613C9E.1010602@php.net> Date: Fri, 18 Oct 2013 14:50:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 References: <1534105.bNC2os93J1@rofl> <525FDAC3.6060103@php.net> <5260D11C.90406@php.net> <52612942.1090707@pthreads.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Posted-By: 80.4.21.210 Subject: Re: [PHP-DEV] Assertions From: krakjoe@php.net (Joe Watkins) On 10/18/2013 01:45 PM, Ferenc Kovacs wrote: > On Fri, Oct 18, 2013 at 2:27 PM, Joe Watkins wrote: > >> On 10/18/2013 01:15 PM, Ferenc Kovacs wrote: >> >> >> >> >> On Fri, Oct 18, 2013 at 1:35 PM, J David wrote: >> >>> On Fri, Oct 18, 2013 at 2:11 AM, Joe Watkins wrote: >>>> You have been provided very good rationale for the use of exceptions to >>>> handle failed assertions, we'd all be grateful if you could stop >>> derailing >>>> the conversation. >>> >>> Wow. Well, if you speak for everyone on this subject then go ahead >>> and implement it. Break all existing code everywhere that uses >>> assert() and catches all exceptions. >>> >>> But: >>> >>> On Thu, Oct 17, 2013 at 6:51 AM, Julien Pauli wrote: >>>> However, there is always the debatte about if a Core feature should >>> throw >>>> an Exception or generate an error. >>> >>> On Thu, Oct 17, 2013 at 7:45 AM, Sebastian Krebs >>> wrote: >>>> Actually if an assertion fails it means, that the application is totally >>>> broken and cannot get recovered. The use case you describe is more like >>>> "validation". >>>> This means, that even if it throws an AssertionException, when you are >>> able >>>> to catch it and recover the process, it means, that "assert" was the >>> wrong >>>> choice >>> >>> On Thu, Oct 17, 2013 at 8:01 AM, Michael Wallner wrote: >>>> I tend to see it the same way. I think PHP's assert is derived from >>>> C's assert, where ASSERT(3) says: >>>> >>>> "... assert() prints an error message to standard error and terminates >>>> the program by calling abort(3) if expression is false ..." >>>> >>>> Where the important part is "terminates the program". >>> >>> On Thu, Oct 17, 2013 at 5:42 PM, Rowan Collins >>> wrote: >>>> Reading through the discussion, I think this point may deserve more >>>> prominence: the problem with throwing an exception is that it is >>> possible >>>> (and probably quite common) to have a catch block which catches *all* >>>> exceptions. Throwing an exception from assert() tangles up any handling >>> you >>>> did want to do for failed assertions with the presumably rather >>> different >>>> handling you want to do for runtime exceptions. >>> >>> are you sure you speak for "all" and that merely expressing an >>> dissenting opinion is "derailing the discussion?" >>> >>> The feature you want is a useful new feature. The only feedback I am >>> offering you is to please consider adding it without breaking existing >>> features, for example by calling it "expect" or "validate" instead of >>> "assert." The word "assert" is not innately magic. Put "see also >>> 'expect'" in the documentation of assert and if your functionality is >>> truly better in all cases, people will naturally move from assert to >>> expect and eventually maybe assert can be deprecated if PHP is really >>> such a high-level language where it is not appropriate to have. The >>> only effect of calling this new functionality "assert" is to break >>> existing code. >>> >>> Not to mention that since this approach does not break BC, doing it >>> this way may allow the implementation of this feature sooner from a >>> release standpoint. >>> >>> Likewise, if you separately want to enhance the diagnostic value of >>> the existing assert functionality without breaking existing code, that >>> would also, I believe, be most welcome. But I don't pretend to speak >>> for all so I cannot say for sure. >>> >>> If your response is "No, you are wrong because I say so," then so be >>> it. You may rely on my lack of further participation in the >>> discussion, per your request. >>> >>> Thanks! >>> >>> -- >>> PHP Internals - PHP Runtime Development Mailing List >>> To unsubscribe, visit: http://www.php.net/unsub.php >>> >>> Hi, >> >> Personally I think that your argument was solid, and I'm fairly sure >> that there are a bunch of code out there with your usecase: >> >> try{ >> // call a bunch of functions, which in turn calls a bunch of functions, >> etc. >> // where one of the functions uses assert() >> } catch(Exception $) { >> // report some generic error somewhere, or retry the calls a couple of >> times before bailing out >> // or do nothing, or throw a different exception with or without setting >> the previous exception to it >> } >> >> those kind of code would potentially swallow/silence the assert violation >> or potentionally show a different error scenario what really happened, and >> leave the developer confused how can he see that kind of error when the >> same code works in production (where the assertions don't run). >> >> using exceptions would also be less flexible than the current solution, >> where you killed assert_options, so introduced a BC break for everybody >> using that. >> >> I also agree that maybe the new one should be added instead of replacing >> the current(hence keeping BC), and later (when we have some feedback, and >> maybe extended the new way with some of the old options) can decide to >> deprecate the "old" assert infrastructure. >> >> -- >> Ferenc Kovács >> @Tyr43l - http://tyrael.hu >> >> The catch all argument is only valid when we are talking about migration, >> and it's only an issue because we are supported a kind of backward >> compatibility by accepting strings - even though there is no need to accept >> strings, at all. >> >> So would it be better then to break compatibility in that way, by removing >> the ability to eval string expressions from the new implementation such >> that migrating users of the current implementation cannot fall into the >> catch all trap because their assertions will pass ?? >> >> Assert options are only there to service the current implementation of >> assertion, the function can be added and generate an E_DEPRECATED, you are >> looking at a working, work in progress not a final implementation of the >> idea. >> >> I don't like the idea of naming it something else, because it isn't >> something else, and that will just be confusing, it is a re-implementation >> of assert at the language level. I'd rather break compatibility in some >> graceful way, or emulate it, than confuse anyone ... >> >> Cheers >> Joe >> >> > agree, but we either have to keep BC(if we want it in the next minor > version), or provide an easy migration path(if we want it in the next major > version). > keeping the name but changing the behavior in a way, which would require a > major code review or removing all assertions which was written with the old > behavior(to make sure you don't have assert_options() call or asserts in > catch all blocks) would be really a bad thing, it would present us as > changing things for the shake of change, and it could even slow/halt > adoptation of the new version. > > > Following on from everything said in IRC and here ... This has now been renamed "expect", the same feature with a different name causes no clashes or problems with BC ... I will write up an RFC today sometime and post it ... Thanks for all the input ... everybody :) Cheers Joe