-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Promote some warnings to Errors in Zend basic functions #5325
Conversation
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 |
There was a problem hiding this comment.
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? 🤔
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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 ^^"
There was a problem hiding this comment.
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 :)
31cee5f
to
5919a62
Compare
There was a problem hiding this 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.
198323d
to
9df93a0
Compare
9df93a0
to
8531fe8
Compare
I didn't know how to handle some of the warnings so I left them be.