[PHP-INTERNALS] Assertions

I like the idea.
However, there is always the debatte about if a Core feature should throw
an Exception or generate an error.
In our current system, they don't throw Exceptions but generate errors.
Also : someone could use a callback, and then make them throw exceptions if
he wants to ; the callback on assertion fail, which IMO is a good feature,
has dissapeared in your patch.

Julien.Pauli

2013/10/17 Julien Pauli <jpauli@php.net>
Me too. And yes, I really use assertions.

Because assertions (or at least the resulting error) should be disabled in
production I'd be fine with an error too.

Morning Julien,

  That was brought up in IRC yesterday, someone pointed out that
generators throw exceptions in some places (from an object method, but
still). So it's not unprecedented, and nobody has provided reason for
the preference to use errors over exceptions ... however, general case
aside:

  Throwing exceptions here makes much more sense than errors, and is the
reason there is no need for callbacks; should you need to control the
flow of execution or take some action because of a failed assertion you
can catch the AssertionException and do that without cluttering ini and
module globals in order to do it.

  In general, of course, an assertion should abort execution, that's a
bit final for PHP and runs contrary to
carry-on-executing-no-matter-what, which we like, on the interweb. But,
should you need to take some action and stop that exception from
bubbling to the surface you should do that with language constructs
try/catch rather than dated callbacks, which don't give you the same
kind of recovery options so easily, they additionally require a new
scope, a new function entry, module globals for support, and ini
settings to configure ...

  That being said, I would rather reimplement the ability to invoke a
callback than scrap the idea all together ... but might we have
something a bit more PHP5 ??

  assert(
   $expression != false ||
   (CONSTANT_MASK & HAS_BITS),
   function(){
    echo "oh rly?"; }
  );

  I still think it's a bit unnecessary, and this:

  try {
   assert($expression != false ||
    (CONSTANT_MASK & HAS_BITS));

  } catch (AssertionException $ex) {
   echo "oh rly?";
  }

  Makes much more sense ... I would rather encourage or even require that
than reimplement callbacks ... they only seemed to exist to service the
current implementation.

  In summary then, I think exceptions make more sense than errors and
callbacks ...

  So what do we think about callbacks and errors now, am I making any
sense ??

Cheers
Joe

2013/10/17 Joe Watkins <krakjoe@php.net>
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

if (!$expression && !(CONSTANT_MASK & HAS_BITS)) {
     echo "oh rly?";
}

/**
  * @var FooObject $foo properly set up FooObject-object
  */
function getByName (FooObject $foo) {
     assert($foo->getName()); // unusable without name. You use this
function wrong. Fix your application!!!
}


While tinking about it: As assertions are a way to describe conditions,
that are valid _in every case_ (or else the application is unrecoverable
broken), it is somehow different from just "exeptional cases", that may be
recovered during runtime. Therefore I'd prefer "the hard with" with errors
and such. I think they are somehow comparable to compile-errors, but during
runtime. Or with a "LogicException", that isn't simply (and without side
effects) solveable by replacing the value of one variable with another one.

If a failed assertion should _just_ abort execution then a callback, or
catch block are not even required.

Never mind the example, the point is that a callback does the same job
as a catch block, without requiring module globals, ini settings, and
supporting functions etc ...

Cheers
Joe

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".


-- off topic --

What I'd love to see, though, would be conditional execution of the
assert block, i.e. if ASSERT_ACTIVE!=true, the expression wouldn't
even be evaluated. So it were usable with expressions instead of
strings in production code. I guess, accepting a callable as first
argument could more or less solve this wish now that we've got
closures...

Sorry Mike, you'll get this twice, just to keep everyone updated ...

By default, php's assert implementation will just generate a warning.

By default, this implementation throws an exception, resulting in
abortion of execution if the exception is not handled ...

I'm still thinking about disabling compilation of the expression ... I'm
not sure it's practical ...

Cheers

(Stupid GMail, didn't reply automatically to the list)
This forces me to write a description and essentially duplicate the
assertion code, while an AssertionException already contains a
meaningful message ("Assertion in foo.php:1337 failed:
isset($_GET["bar"])!=TRUE").

Marco

This makes sense.

However, I still continue to think that assertions should stay assertions,
aka : run time fatal errors when enabled.
For me, there is nothing to "catch", except the fact that the program has
stopped somewhere, it tells you where, and why, and you have to fix it :
that's an assertion (for me).
Enabling or disabling it with an INI setting is just all right.

Having assert() with exception makes the code much more heavy for me.

<?php

assert(MYCONST == "bar");
foo();

In the code above, I dont expect to catch any exception. Like in C, where
usually assert() is just a macro bound to some #define , I want assertion
to clearly be as invisible as possible when I disable them.
With a catch, that make the code look heavier.

Those were my opinions :-)


Julien.Pauli

Yes! And Coredump! And a debugger that lets me load that later +
examine stack and all other state!

BTW, http://www.php.net/manual/de/debugger-about.php link to DBG
leads to a nonexisting page http://www.php-debugger.com/dbg/

best regards
   Patrick

Hm seriously, optional (sort of) coredumps on fatals would be cool.

AssertionExceptions have a stack trace ...

Obviously, coredumps don't make a whole lot of sense for PHP
programmers, but I guess you knew that, which is why you say sort of ...

Being that ZEND_ASSRT is an opcode, xdebug could override it and provide
the functionality you want from failed assertions, it is out of the
scope of core I think ...

Cheers
Joe

Assertions have three defining characteristics:

1) They are (potentially inefficient) checks run in
debug/non-production that completely disappear when run in production.
2) When they pass, they produce no side effects.
3) When an assertion fails, which can only ever happen in
debug/non-production, the program dies.

Each component of this triad interacts with the others to make
assertions something different than the other tools in the debugging
toolbox, which is why they have value.

Talking about catching exceptions thrown by an assertion failure seems
very contrary to that, because even if PHP is hardcoded to completely
ignore everything between "assert" and ";" when parsing in production,
any attempt to catch assertion-related exceptions won't get the same
treatment and will still be present in the production code. This can
cause a change in behavior, particularly in the case of catching all
exceptions, and encourages the generation of side effects (like trying
to recover an error) from an assertion test which will no longer be
performed when assertions are disabled for production.

If you want a concise easy-to-write test that throws an exception if
it's not true and runs in production code in catchable way that
doesn't abort the app, that is a great idea. But it is not an
assertion. It is also already easy to do. Just write:

function Expect($shouldBeTrue, $message) {
   if (!$shouldBeTrue) throw new FMLException($message);
}

Expect($myObject->internalState == OBJECT_STATE_VALID, "object state
is not valid");

Customize as needed with your favorite exception type.

Maybe there is some reason why a variant of this should be in the
language core, although that's not immediately clear, but it seems
really hard to justify redefining the solid and well-understood
concept of assertions to do it.

Thanks!

Nobody is arguing that assertions should be used as a control-flow
structure. It's still most useful to implement them as exceptions. Apart
from being a lot better integrated than fatal errors (in particular they
have such fancy things like stacktraces...) exceptions can also be
gracefully handled where necessary. E.g. PHPUnit will catch the exception
and tell you that an assertion failed, but it will still be able to
continue running the rest of the testsuite. A framework (or front
controller in general) can easily catch the exception at the top level and
display an appropriate debugging page (rather than doing ugly
register_shutdown_function hacks that are necessary to have even remotely
graceful handling of fatals). A deamon or server will be able to process
further requests, even if one particular had an assertion failure.
Assertions shouldn't be used for control flow, but it should still be
possible to handle them gracefully. Exceptions offer that, fatals do not.

Nikita

If it is useful to handle an assertion, then it wasn't an assertion.
The minute you talk about "handling" assertions they become a control
flow structure and cease to be assertions.
Those are three great examples of how assertions are frequently misused.

There are *many* cases where assertions are not useful. That means
use a different tool in those cases (like an exception), not that
assertions should be made into shorthand for exception throwing.

That functionality is fine to have, just call it something else.

Thanks!

I would greatly appreciate a more detailed explanation as to how these
examples abuse assertions. In particular the first two seem very reasonable
to me, but I'm open to being convinced otherwise.

Nikita

All three use the failure of an assertion to alter the flow of
execution of the program. Essentially you wrote, "Nobody is arguing
that assertions should be used as a control-flow structure" and then
provided three examples of using assertions to do exactly that.

All three cause unexpected behavior if the asserted condition is not
true and assertions are disabled because the condition was not
evaluated.

In all three of those examples, the behavior you want is easily
obtained by throwing an exception.

In fact, what some people might do in those situations is:

assert($myObject->internalState == OBJECT_STATE_VALID, "internal
object state not valid");
if ($myObject->internalState != OBJECT_STATE_VALID)
   throw FMLException("internal object state not valid");

(In this case, you would probably do the PHPUnit stuff with assertions
disabled.)

Assertions are a debugging/development check designed to facilitate
proof of correctness. They are not present or used in production
code. There is no danger that a failed assertion will ever cause a
production app to abort. They serve one purpose, which is to bring
things to a screeching halt when somethat that should never, ever
happen has happened. And their value comes *because* they are
independent of methods used to handle potentially-recoverable errors.

Why do we need a new way to throw an exception?

Why do we need to mess up another tool that has a different purpose to do it?

What's wrong with writing the three-line function one time to get the
behavior you want?

It's totally reasonable to tie in to the stack-trace generating code
to provide as much information as possible about why an assertion
failed, similar to what exceptions do. But assertions must not be
catchable through any normal mechanism, and the minute code execution
resumes after a check fails, that check is absolutely a control-flow
structure, and it is not an assertion.

Thanks!

Sorry, I still don't understand what you're trying to say. Ignoring my last
example, both PHPUnit and a "debug page" from the framework are clearly for
debugging/development. Both examples just take the failed assertion and
present it in a more appropriate form.

I am most certain that you do *NOT* want to abort the whole unit test run
because of an assertion failure in one single test. Tests are supposed to
continue running even if individual ones fail. There is nothing inherently
different between a test that failed because of an assertion and a test
that failed because it threw an unexpected exception or returned a wrong
value. In either case you want to see a test failure and continue to run.
Just imagine how terrible it would be if the PHP test suite would stop
running on the first test failure (of 12000 tests).

Similarly, showing a friendly debugging page when an assertion fails does
not seem to go against your idea of assertions. It just changes the
presentation form to simplify debugging the cause by specifying additional
information that is known to the framework (but not to the assertion
itself).

Those questions have a very simple answer: You can't disable code throwing
exceptions ;) Assertions can be disabled.

Nikita

You may be using asserts to test much too complicated concepts.

Assertions are a *very* low-level feature. The classic example of an
assert is to check that expectations are valid in some function that
gets called in a loop 20,000,000 times and takes 6 minutes to run if
you check the conditions and 3 seconds if you don't.

If you are using assert to check that the session data is valid or
that the MySQL connection is valid or something, that's not good. If
there's an error message, it wasn't an assertion. If it's
recoverable, it wasn't an assertion.
No, the code that throws "assertion exceptions" can be disabled. The
code that catches them cannot.

The value that assertions provide is the independent error handling
channel. If you do not want the value that assertions provide, don't
use them.

If you want to make that error handling channel more robust and
useful, go nuts, that would be great.

If you want disable-able exception checks that can be skipped based on
a global setting, and for some reason testing the global setting
before testing the condition and throwing the exception is offensive
to you, find a good name for this new feature and propose that.

But please don't screw up existing code for people that use assertions
properly by redefining them as exceptions.

The behavior you're describing is trivial to implement at the user
level, and it's still not clear why it belongs in the language core or
why it needs to alter other, different, existing behavior.

Thanks!

Assertions in PHP are whatever we implement them to be, there isn't
really any low level features in PHP, it's a high level programming
language where you would not expect to find any.

By default, a failed assertion in PHP results in an E_WARNING being
raised, this is not "using assertions properly", this is just generating
warnings. Assertions, written originally for PHP4, are currently
supported by other user land functions, module globals, callbacks and
ini settings. You can only evaluate strings, you cannot use actual
expressions, you cannot use anonymous functions.

We are not implementing assert() as it is in C, we are implementing an
assert() that is useful in PHP5. One that does not require user land
functions, module globals and a bunch of ini settings.
One that does allow you to use expressions, and annonymous functions.

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.

Cheers
Joe

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:
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!

You are talking about the current implementation of assert as if it is
an actual implementation of assert, and it is not. This leads me to
think you didn't actually look at the manual, or the current
implementation, before you started to reel about how assert should and
does work - even though, it doesn't.

So I think I see what you are worried about, catch all blocks ??
seriously ?? we are meant to prepare for that ??

  Julien I responded too, I don't know if he agrees or not, but I've
given him a good explanation of why we should use exceptions.

  Mike: PHP's assert is not derived from C, or isn't deployed as such; by
default it is a way to generate E_WARNING's, which is the wrong default
for an implementation of assert. Furthermore, it's a serious restriction
that you can only use string assertions, one that cannot be worked
around from an extension.

  Sebastian: That's what assert does in C, it's not useful to exit from a
PHP program without at least the opportunity to deal with the error,
usually that means an error page, or some kind of action, but not
necessarily continuing with execution. In C a coredump can be generated
and that it useful to the prgorammer, in PHP we have no such thing, we
must have the option to hand back control, the old implementation does
this with callbacks and this implementation with catch blocks.

  Rowan: it is not our responsibility to prepare Zend for bad
programmers; catching all is a horrible horrible thing to do, I don't
think we should be encouraged to restrict what the engine can do in
preparation for poor programming.

I didn't mean to be rude, I meant to be to the point, I don't actually
have the time to sit here and write 5 essays a day, to a single
individual, to explain what should be obvious.

I realise this implementation breaks some compatibility, how much is
retained is open for discussion, if we can just stop talking about what
assert is meant to be or what it isn't meant to be. It is what we make
it, and we are replacing an implementation, so if you used assertion and
callbacks before, you need an option to do the same thing and catch
happens to cover it. I really don't know why you are talking about
breaking compatibility and aborting execution without recourse in the
same breath ...

I think some are thinking that we are implementing access to actual
assert() functionality, we are not doing that, we are implementing a
higher level assert, suitable in PHP, for PHP.

Cheers
Joe

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.

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

The new implementation doesn't _require_ that you control the flow of
execution but _allows_ you to, better than callbacks did.

It is not practical to try and implement assert() as we know it in C, an
aborted PHP program is not useful to the user, there is no coredump,
there is nothing useful we can do other than pass control of execution
back to the program where it is prepared to deal with it ...

Does this new exception derive from Exception? (Hint: It does.)

Is it caught by set_exception_handler() ?

Then it requires you to control the flow of execution.
Assertions do not abort programs in front of the user, because they
are turned off. They abort programs in front of the developer.
The suggestion to have assertion failure generate an exception-like
stack trace was an excellent one. The "PHP core dump" idea was pretty
cool too, though one can only imagine how much work that would be.

Beyond that, you are essentially arguing to remove assert() entirely,
since the functionality you describe already exists. It's called
throwing exceptions and the presented rationale for redefining
language features to create new ways to throw them is unsatisfactory
to me.

It's hard not to notice that the fundamental question keeps getting
avoided: if this is the behavior you want, what's wrong with throwing
an exception? What new capability is being created that justifies
changing the language and sacrificing the old capability?

Thanks!

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.

The current implementation allows you to define a callback, and to
choose whether assertions are fatal, which means that unit test
frameworks and fancy-output wrappers can intercept assertions *without
affecting other parts of the code*. The only way to do that with catch
blocks would be to have an exception which didn't descend from Exception.

You could always throw a secondary exception from the catch-block.

Marco

It's not so much a question of whether it would be *possible* to write
code which handled such situations, as whether it is reasonable to
*expect* people to do so: as soon as assert() started throwing
exceptions, all code which uses assert() would have to be audited to see
what catch blocks the AssertExceptions would fall into, and add extra
boilerplate to explicitly re-throw them.

I agree with J David to the extent that whatever behaviour assert()
should have, it should be *distinct* from other types of error checking.
Having assertions be caught by a catch(Exception) block reduces that
distinction while increasing the BC break.

Meanwhile, what is the actual case for catching the assertion in a code
block rather than registering a failure callback? So far, it seems to be
a case of "callbacks feel a bit old-fashioned" - the uses cases people
have mentioned of Unit Tests and pretty debug output are both adequately
covered by a simple callback. Is there a situation where you would, for
instance, want to catch an AssertionException, do something, and then
re-throw it to an outer block?

In addition to all that you've said, I should point out that both Java
and Python have an assert statement which raises an exception, which
happens to be called AssertionError in both languages.

A quick search on GitHub shows quite a lot of people catching
AssertionErrors in unit tests.

And in Python you can subclass an int. This does not make that a good
idea for PHP. (Java only wishes it could subclass int, hence
Integer.)

In Java, for better or for worse, every error is an exception. This
does not make that a good idea for PHP.

(Shrill voice) "If all your friends jumped off a bridge…"

Should require($myFile) throw an exception too? (Some people, perhaps
many, would say yes.) What about parse errors in a file include()d at
runtime?

Thanks!

Well, I'd ask myself *why* they all jumped. Since they are not dumb,
there's likely a good reason for it. Maybe it's a wooden bridge and it's on
fire? In that case, sure, I'd jump right after them ;)

Yes, require() should throw an exception from a modern point of view. It
doesn't, because it was introduced at a time where we had no exceptions.
Parse errors should theoretically also be exceptions, but with the current
compiler this is simply technically infeasible. Actually "technical
infeasibility" is the only sane reason I can think of to throw a fatal
error.

Anyway, I won't continue this pointless discussion, doesn't seem like we're
getting anywhere with it.

Nikita

I don't have a strong opinion on this, but mind that by being based on
eval the assert expression can use features from a different version of
PHP. Something like

    if (PHP_VERSION_ID >= 50300) {
        assert('foo\\bar::baz == 42');
    }

the new form

    if (PHP_VERSION_ID >= 50300) {
        assert foo\\bar::baz == 42;
    }

would give a parse error on older versions.

(Yes, I'm using 5.3 and <5.3 as example where the new form would fail
anyways, feel free to use PHP_VERSION_ID >= 51200 and PHP 5.26.0
features)
I think this is bad for migration of code. For one it is impossible to
have a code base with asserts working in both versions. For two it
silently leads to completely different behavior. Changing this needs
some clever strategy allowing migration.

johannes

Okay, so if the expression used is a string, it is now evaluated, this
provides some backward compatibility and the possibility to avoid
compilation of the expression at compile time...

Better ??

Cheers
Joe