Skip to content

Show enum cases in errors #14496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Show enum cases in errors #14496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Zend/tests/enum_in_stack_trace.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Enum case is shown in stack trace
--FILE--
<?php

enum Foo {
case Bar;
}

function test($enum) {
throw new Exception();
}

test(Foo::Bar);

?>
--EXPECTF--
Fatal error: Uncaught Exception in %s:%d
Stack trace:
#0 %s(%d): test(Foo::Bar)
#1 {main}
thrown in %s on line %d
21 changes: 21 additions & 0 deletions Zend/tests/match/unmatched_enum.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
Match errors show unmatched enum case
--FILE--
<?php

enum Foo {
case Bar;
case Baz;
}

try {
match (Foo::Bar) {
Foo::Baz => 42,
};
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECT--
Unhandled match case Foo::Bar
3 changes: 1 addition & 2 deletions Zend/zend_exceptions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,7 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */

ZVAL_DEREF(arg);

if (Z_TYPE_P(arg) <= IS_STRING) {
smart_str_append_scalar(str, arg, EG(exception_string_param_max_len));
if (smart_str_append_zval(str, arg, EG(exception_string_param_max_len)) == SUCCESS) {
smart_str_appends(str, ", ");
} else {
switch (Z_TYPE_P(arg)) {
Expand Down
6 changes: 2 additions & 4 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,14 +873,12 @@ ZEND_COLD zend_never_inline void zend_magic_get_property_type_inconsistency_erro
ZEND_COLD void zend_match_unhandled_error(const zval *value)
{
smart_str msg = {0};

if (Z_TYPE_P(value) <= IS_STRING) {
smart_str_append_scalar(&msg, value, EG(exception_string_param_max_len));
if (smart_str_append_zval(&msg, value, EG(exception_string_param_max_len)) == SUCCESS) {
/* Nothing to do. */
} else {
Comment on lines +876 to 878
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe?

Suggested change
if (smart_str_append_zval(&msg, value, EG(exception_string_param_max_len)) == SUCCESS) {
/* Nothing to do. */
} else {
if (smart_str_append_zval(&msg, value, EG(exception_string_param_max_len)) == FAILURE) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, that looks correct. We only want to emit "of type ..." if smart_str_append_zval() can't handle the value. Note that it intentionally does not handle some values like arrays, because they seem to be handled differently depending on the context. It would be nice if it could be unified in the future, but my intention is not to break BC here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry. I misread your suggestion. :) Yeah, I'll change it!

smart_str_appendl(&msg, "of type ", sizeof("of type ")-1);
smart_str_appends(&msg, zend_zval_type_name(value));
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line should be reverted

smart_str_0(&msg);

zend_throw_exception_ex(
Expand Down
15 changes: 15 additions & 0 deletions Zend/zend_smart_str.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <zend.h>
#include "zend_smart_str.h"
#include "zend_smart_string.h"
#include "zend_enum.h"

#define SMART_STR_OVERHEAD (ZEND_MM_OVERHEAD + _ZSTR_HEADER_SIZE + 1)
#define SMART_STR_START_SIZE 256
Expand Down Expand Up @@ -221,3 +222,17 @@ ZEND_API void ZEND_FASTCALL smart_str_append_scalar(smart_str *dest, const zval
EMPTY_SWITCH_DEFAULT_CASE();
}
}

ZEND_API zend_result ZEND_FASTCALL smart_str_append_zval(smart_str *dest, const zval *value, size_t truncate)
{
if (Z_TYPE_P(value) <= IS_STRING) {
smart_str_append_scalar(dest, value, truncate);
} else if (Z_TYPE_P(value) == IS_OBJECT && (Z_OBJCE_P(value)->ce_flags & ZEND_ACC_ENUM)) {
smart_str_append(dest, Z_OBJCE_P(value)->name);
smart_str_appends(dest, "::");
smart_str_append(dest, Z_STR_P(zend_enum_fetch_case_name(Z_OBJ_P(value))));
} else {
return FAILURE;
}
return SUCCESS;
}
1 change: 1 addition & 0 deletions Zend/zend_smart_str.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ ZEND_API void smart_str_append_printf(smart_str *dest, const char *format, ...)
ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
ZEND_API void ZEND_FASTCALL smart_str_append_escaped_truncated(smart_str *str, const zend_string *value, size_t length);
ZEND_API void ZEND_FASTCALL smart_str_append_scalar(smart_str *str, const zval *value, size_t truncate);
ZEND_API zend_result ZEND_FASTCALL smart_str_append_zval(smart_str *dest, const zval *value, size_t truncate);
END_EXTERN_C()

static zend_always_inline size_t smart_str_alloc(smart_str *str, size_t len, bool persistent) {
Expand Down
12 changes: 2 additions & 10 deletions ext/reflection/php_reflection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,8 +647,8 @@ static zval *get_default_from_recv(zend_op_array *op_array, uint32_t offset) {
}

static int format_default_value(smart_str *str, zval *value) {
if (Z_TYPE_P(value) <= IS_STRING) {
smart_str_append_scalar(str, value, SIZE_MAX);
if (smart_str_append_zval(str, value, SIZE_MAX) == SUCCESS) {
/* Nothing to do. */
} else if (Z_TYPE_P(value) == IS_ARRAY) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this branch still be reached?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, smart_str_append_zval() does not handle arrays atm, amongst other things.

zend_string *str_key;
zend_long num_key;
Expand All @@ -675,14 +675,6 @@ static int format_default_value(smart_str *str, zval *value) {
format_default_value(str, zv);
} ZEND_HASH_FOREACH_END();
smart_str_appendc(str, ']');
} else if (Z_TYPE_P(value) == IS_OBJECT) {
/* This branch may only be reached for default properties, which don't support arbitrary objects. */
zend_object *obj = Z_OBJ_P(value);
zend_class_entry *class = obj->ce;
ZEND_ASSERT(class->ce_flags & ZEND_ACC_ENUM);
smart_str_append(str, class->name);
smart_str_appends(str, "::");
smart_str_append(str, Z_STR_P(zend_enum_fetch_case_name(obj)));
} else {
ZEND_ASSERT(Z_TYPE_P(value) == IS_CONSTANT_AST);
zend_string *ast_str = zend_ast_export("", Z_ASTVAL_P(value), "");
Expand Down