Skip to content

Promote some warnings to Errors in Zend basic functions #5325

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.

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

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Mar 29, 2020

I didn't know how to handle some of the warnings so I left them be.

Warning: func_get_arg(): The argument number should be >= 0 in %s on line %d
bool(false)
func_get_arg(): Argument #1 ($arg_num) must be greater than or equal to 0
func_get_arg(): Argument 0 not passed to function
Copy link
Member

Choose a reason for hiding this comment

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

Is this zero-indexing how it seems? 🤔

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, because the argument list starts from 0 for func_get_arg() kinda odd but it's documented.

@@ -1480,6 +1483,8 @@ ZEND_FUNCTION(restore_exception_handler)
ZVAL_COPY_VALUE(&EG(user_exception_handler), tmp);
zend_stack_del_top(&EG(user_exception_handlers));
}

// TODO Change to void
Copy link
Member

Choose a reason for hiding this comment

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

@carusogabriel if you are interested :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Some of them only work because I removed the false returns by promoting the warnings to error ^^"

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I wanted to mention this, too :)

@Girgias Girgias force-pushed the zend-built-in-function-warning-to-error branch 2 times, most recently from 31cee5f to 5919a62 Compare March 30, 2020 02:59
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Looks pretty good, some nits.

@Girgias Girgias force-pushed the zend-built-in-function-warning-to-error branch 2 times, most recently from 198323d to 9df93a0 Compare March 30, 2020 15:08
@Girgias Girgias force-pushed the zend-built-in-function-warning-to-error branch from 9df93a0 to 8531fe8 Compare March 31, 2020 12:44
@php-pulls php-pulls closed this in 55a3e5b Mar 31, 2020
@Girgias Girgias deleted the zend-built-in-function-warning-to-error branch March 31, 2020 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants