On Mon, 2004-02-16 at 09:35, Andi Gutmans wrote:
> Hey,
> 
> Marcus didn't mean it adds complexity to the code but to PHP.

I can't see how it would: For those relying on the built-in base class,
it won't change a single thing.

For those wanting their own exception (or to be more correct: _class_)
hierarchy (me, for example, or Ferdinand, or - if I understood him
correctly - Sterling, which makes three and is therefore more than "one
in a million" you mentioned:)) it opens up possibilities and adds
flexibility to the language. 

Flexibility  is a point in which PHP has always been very good: No
enforcements on types, classes, solutions. Everyone is welcome to and is
actually creating amazing frameworks (PEAR, binary cloud, Horde, to name
a few of the big ones) _ontop_ of the core of PHP / the Zend Engine.

> I think the arguments given on this list for why we need this interface 
> were mainly academic 

Hrm, so my argument about Object::getClass() was academic? I actually
never finished my degree:)

> and an attempt of people familiar with Java to copy 
> the hierarchy.

A lot of people have base classes and let all of their other classes
extend them. I don't think this is specifically related to Java.

Of course, I know how to work around this - by duplicating code into
subclasses of exceptions, by simply having a global function or writing
an extension which gets rid of the exception class after Engine startup,
but wouldn't these solutions be rather "unclean" and wasn't your
argument for this that it is a "clean" solution?

Look at it the other way around: Would anybody be investing such an
amount of time into arguing (which, ATM, I am) *if* we had the interface
solution?

> The actual times you will need such an interface is quite 
> rare and I don't think it's worth the complexity for the one in a million 
> purist guy (i.e. people who want all their classes to inherit from a 
> CObject including exceptions).

Again, I don't see any complexity arising from this: The average user
would simply use "catch (IException $e)" instead of "catch (Exception
$e)" and have what he/she wants: All exceptions are being caught.

The "purist guy" (me, in this case) would use the same semantics *and*
be able to have all of the purist and academic stuff he wants:)

> Andi
> 
> P.S. - BTW, why not call it IException (or ExceptionInterface) instead of 
> Throwable? I think for people who don't know Java, it makes more sense.

Agreed. An updated patch is attached.

- Timm
Index: Zend/zend_exceptions.c
===================================================================
RCS file: /repository/ZendEngine2/zend_exceptions.c,v
retrieving revision 1.58
diff -u -r1.58 zend_exceptions.c
--- Zend/zend_exceptions.c	12 Feb 2004 10:38:14 -0000	1.58
+++ Zend/zend_exceptions.c	16 Feb 2004 21:34:04 -0000
@@ -28,6 +28,7 @@
 #include "zend_interfaces.h"
 
 zend_class_entry *default_exception_ce;
+zend_class_entry *default_iexception_ce;
 static zend_object_handlers default_exception_handlers;
 ZEND_API void zend_throw_exception(zend_class_entry *exception_ce, char *message, long code TSRMLS_DC);
 
@@ -389,11 +390,11 @@
 	}
 
 	if (Z_STRLEN(message) > 0) {
-		len = zend_spprintf(&str, 0, "exception '%s' with message '%s' in %s:%ld\nStack trace:\n%s", 
+		len = zend_spprintf(&str, 0, "exception '%s' with message '%s' in %s:%ld\nStack trace:\n%s\n", 
 							Z_OBJCE_P(getThis())->name, Z_STRVAL(message), Z_STRVAL(file), Z_LVAL(line), 
 							(trace && Z_STRLEN_P(trace)) ? Z_STRVAL_P(trace) : "#0 {main}\n");
 	} else {
-		len = zend_spprintf(&str, 0, "exception '%s' in %s:%ld\nStack trace:\n%s", 
+		len = zend_spprintf(&str, 0, "exception '%s' in %s:%ld\nStack trace:\n%s\n", 
 							Z_OBJCE_P(getThis())->name, Z_STRVAL(file), Z_LVAL(line), 
 							(trace && Z_STRLEN_P(trace)) ? Z_STRVAL_P(trace) : "#0 {main}\n");
 	}
@@ -422,6 +423,14 @@
  * And never try to change the state of exceptions and never implement anything
  * that gives the user anything to accomplish this.
  */
+static zend_function_entry default_iexception_functions[] = {
+	ZEND_ABSTRACT_ME(exception, getMessage, NULL)
+	ZEND_ABSTRACT_ME(exception, getFile, NULL)
+	ZEND_ABSTRACT_ME(exception, getLine, NULL)
+	ZEND_ABSTRACT_ME(exception, __toString, NULL)
+	{NULL, NULL, NULL}
+};
+
 static zend_function_entry default_exception_functions[] = {
 	ZEND_ME(exception, __clone, NULL, ZEND_ACC_PRIVATE|ZEND_ACC_FINAL)
 	ZEND_ME(exception, __construct, NULL, 0)
@@ -439,12 +448,23 @@
 {
 	zend_class_entry ce;
 
-	INIT_CLASS_ENTRY(ce, "exception", default_exception_functions);
+	INIT_CLASS_ENTRY(ce, "IException", default_iexception_functions);
+	default_iexception_ce = zend_register_internal_class(&ce TSRMLS_CC);
+	default_iexception_ce->ce_flags = ZEND_ACC_ABSTRACT | ZEND_ACC_INTERFACE;
+
+	INIT_CLASS_ENTRY(ce, "Exception", default_exception_functions);
 	default_exception_ce = zend_register_internal_class(&ce TSRMLS_CC);
 	default_exception_ce->create_object = zend_default_exception_new; 
 	memcpy(&default_exception_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
 	default_exception_handlers.clone_obj = NULL;
 
+	/* Implement the IException interface */
+	{
+		zend_uint num_interfaces = ++default_exception_ce->num_interfaces;
+		default_exception_ce->interfaces = (zend_class_entry **) realloc(default_exception_ce->interfaces, sizeof(zend_class_entry *) * num_interfaces);
+		default_exception_ce->interfaces[num_interfaces - 1] = default_iexception_ce;
+	}
+
 	zend_declare_property_string(default_exception_ce, "message", sizeof("message")-1, "", ZEND_ACC_PROTECTED TSRMLS_CC);
 	zend_declare_property_string(default_exception_ce, "string", sizeof("string")-1, "", ZEND_ACC_PRIVATE TSRMLS_CC);
 	zend_declare_property_long(default_exception_ce, "code", sizeof("code")-1, 0, ZEND_ACC_PROTECTED TSRMLS_CC);
@@ -470,8 +490,8 @@
 
 	MAKE_STD_ZVAL(ex);
 	if (exception_ce) {
-		if (!instanceof_function(exception_ce, default_exception_ce TSRMLS_CC)) {
-			zend_error(E_NOTICE, "Exceptions must be derived from exception");
+		if (!instanceof_function(exception_ce, default_iexception_ce TSRMLS_CC)) {
+			zend_error(E_NOTICE, "Exceptions must implement the IException interface");
 			exception_ce = default_exception_ce;
 		}
 	} else {
@@ -511,43 +531,44 @@
 ZEND_API void zend_exception_error(zval *exception TSRMLS_DC)
 {
 	zend_class_entry *ce_exception = Z_OBJCE_P(exception);
-	if (instanceof_function(ce_exception, default_exception_ce TSRMLS_CC)) {
+	if (!instanceof_function(ce_exception, default_iexception_ce TSRMLS_CC)) {
+		zend_error(E_ERROR, "Uncaught exception '%s'", ce_exception->name);
+	} else {
 		zval *str, *file, *line;
 		zval *old_exception = EG(exception);
 
 		EG(exception) = NULL;
-		
+
+		/* Call the methods __toString(), getFile() and getLine(), the IException 
+		 * interface ensures they exist.
+		 */
 		zend_call_method_with_0_params(&exception, ce_exception, NULL, "__tostring", &str);
-		if (!EG(exception)) {
-			if (Z_TYPE_P(str) != IS_STRING) {
-				zend_error(E_WARNING, "%s::__toString() must return a string", ce_exception->name);
-			} else {
-				zend_update_property_string(default_exception_ce, exception, "string", sizeof("string")-1, EG(exception) ? ce_exception->name : Z_STRVAL_P(str) TSRMLS_CC);
-			}
-		}
-		zval_ptr_dtor(&str);
-	
+		zend_call_method_with_0_params(&exception, ce_exception, NULL, "getfile", &file);
+		zend_call_method_with_0_params(&exception, ce_exception, NULL, "getline", &line);
+
+		/* The method call might throw an exception itself. There is no reproducing
+		 * script to accomplish this behaviour, as throwing an exception from (e.g.)
+		 * __toString() will result in a fatal error (Exception thrown without a 
+		 * stack frame in Unknown on line 0) but we'll handle this anyways
+		 * just to be sure.
+		 */
 		if (EG(exception)) {
-			/* do the best we can to inform about the inner exception */
-			if (instanceof_function(ce_exception, default_exception_ce TSRMLS_CC)) {
-				file = zend_read_property(default_exception_ce, EG(exception), "file", sizeof("file")-1, 1 TSRMLS_CC);
-				line = zend_read_property(default_exception_ce, EG(exception), "line", sizeof("line")-1, 1 TSRMLS_CC);
-			} else {
-				file = NULL;
-				line = NULL;
-			}
-			zend_error_va(E_WARNING, file ? Z_STRVAL_P(file) : NULL, line ? Z_LVAL_P(line) : 0, "Uncaught %s in exception handling during call to %s::__tostring()", Z_OBJCE_P(EG(exception))->name, ce_exception->name);
+			zval_ptr_dtor(&str);
+			zval_ptr_dtor(&file);
+			zval_ptr_dtor(&line);
+
+			/* Restore the exception and bail */
+			EG(exception) = old_exception;
+			zend_error(E_ERROR, "Uncaught %s in exception handling for %s", Z_OBJCE_P(EG(exception))->name, ce_exception->name);
 		}
 
-		str = zend_read_property(default_exception_ce, exception, "string", sizeof("string")-1, 1 TSRMLS_CC);
-		file = zend_read_property(default_exception_ce, exception, "file", sizeof("file")-1, 1 TSRMLS_CC);
-		line = zend_read_property(default_exception_ce, exception, "line", sizeof("line")-1, 1 TSRMLS_CC);
+		convert_to_string(str);
+		convert_to_string(file);
+		convert_to_long(line);
 
+		/* Restore the exception and bail */
 		EG(exception) = old_exception;
-
-		zend_error_va(E_ERROR, Z_STRVAL_P(file), Z_LVAL_P(line), "Uncaught %s\n  thrown", Z_STRVAL_P(str));
-	} else {
-		zend_error(E_ERROR, "Uncaught exception '%s'", ce_exception->name);
+		zend_error_va(E_ERROR, Z_STRVAL_P(file), Z_LVAL_P(line), "Uncaught %s thrown", Z_STRVAL_P(str));
 	}
 }
 
@@ -562,8 +583,8 @@
 
 	exception_ce = Z_OBJCE_P(exception);
 
-	if (!exception_ce || !instanceof_function(exception_ce, default_exception_ce TSRMLS_CC)) {
-		zend_error(E_ERROR, "Exceptions must valid objects that are derived from class Exception");
+	if (!exception_ce || !instanceof_function(exception_ce, default_iexception_ce TSRMLS_CC)) {
+		zend_error(E_ERROR, "Exceptions must valid objects that implement the IException interface");
 	}
 	zend_throw_exception_internal(exception TSRMLS_CC);
 }
Index: Zend/zend_execute.c
===================================================================
RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.612
diff -u -r1.612 zend_execute.c
--- Zend/zend_execute.c	12 Feb 2004 15:23:06 -0000	1.612
+++ Zend/zend_execute.c	16 Feb 2004 21:34:06 -0000
@@ -2854,21 +2854,12 @@
 
 int zend_catch_handler(ZEND_OPCODE_HANDLER_ARGS)
 {
-	zend_class_entry *ce;
-
 	/* Check whether an exception has been thrown, if not, jump over code */
 	if (EG(exception) == NULL) {
 		SET_OPCODE(&op_array->opcodes[opline->extended_value]);
 		return 0; /* CHECK_ME */
 	}
-	ce = Z_OBJCE_P(EG(exception));
-	if (ce != EX_T(opline->op1.u.var).class_entry) {
-		while (ce->parent) {
-			if (ce->parent == EX_T(opline->op1.u.var).class_entry) {
-				goto exception_should_be_taken;
-			}
-			ce = ce->parent;
-		}
+	if (!instanceof_function(Z_OBJCE_P(EG(exception)), EX_T(opline->op1.u.var).class_entry TSRMLS_CC)) {
 		if (opline->op1.u.EA.type) {
 			zend_throw_exception_internal(NULL TSRMLS_CC);
 			NEXT_OPCODE();
@@ -2877,7 +2868,6 @@
 		return 0; /* CHECK_ME */
 	}
 
-exception_should_be_taken:
 	zend_hash_update(EG(active_symbol_table), opline->op2.u.constant.value.str.val,
 		opline->op2.u.constant.value.str.len+1, &EG(exception), sizeof(zval *), (void **) NULL);
 	EG(exception) = NULL;

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to