Skip to content

Cleanup of remaning E_STRICT - #4401

Closed
Girgias wants to merge 1 commit into
php:PHP-7.4from
Girgias:e-strict-cleanup
Closed

Cleanup of remaning E_STRICT#4401
Girgias wants to merge 1 commit into
php:PHP-7.4from
Girgias:e-strict-cleanup

Conversation

@Girgias

Copy link
Copy Markdown
Member

Removed usage of E_STRICT in a couple of places by using lxr [1]
I've converted the last usages to E_NOTICE however as @nikic is the author of the RFC to reclassify them [2] could you have a look at them?

Removed the E_STRICT in the default error handling mode as this shouldn't trigger anything more now.

Not sure if we should keep the mention of E_STRICT in the INI files, I've removed them as they seem pointless ATM.
The constant still exists in the Zend engine which we may want to remove in PHP 8.

[1] https://lxr.room11.org/search?project=php-src%407.4&q=E_STRICT&defs=&refs=&path=&hist=&type=
[2] https://wiki.php.net/rfc/reclassify_e_strict

@nikic

Copy link
Copy Markdown
Member

Can you please separate out the two changes that affect where E_STRICT is thrown?

@Girgias

Copy link
Copy Markdown
MemberAuthor

Can you please separate out the two changes that affect where E_STRICT is thrown?

Done :)

Comment threadext/pdo/pdo_stmt.c Outdated
Comment threadext/pdo_mysql/tests/pdo_mysql_subclass.phpt Outdated
Comment threadZend/tests/ErrorException_getSeverity.phpt Outdated
Comment threadZend/tests/bug33771.phpt Outdated
Comment threadZend/tests/error_reporting02.phpt Outdated
Comment threadZend/tests/error_reporting03.phpt Outdated
Comment threadZend/tests/exception_004.phpt Outdated
Comment threadext/soap/tests/bugs/bug46427.phpt Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be kept with E_STRICT replaced with something else, I think. Possibly just a dummy value (if hex is supported, something like 0x100000), as this is a parsing test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

There are a bunch of whitespaces after are those part of the test too?

Comment threadext/standard/tests/mail/mail_basic_alt1-win32.phpt Outdated
Comment threadphp.ini-development Outdated
Comment threadtests/func/011.phpt Outdated
@Girgias
Girgiasforce-pushed the e-strict-cleanup branch 2 times, most recently from 26a1307 to a1053d0CompareJuly 18, 2019 14:20
@GirgiasGirgias closed this Jul 18, 2019
@GirgiasGirgias reopened this Jul 18, 2019
@Girgias

Girgias commented Jul 18, 2019

Copy link
Copy Markdown
MemberAuthor

Closed and Reopened to restart CI after I messed up my rebase and it didn't pick up on the quick subsequent push

Edit: nope just another file I missed after i f* the rebase onto upstream >__>

@Girgias

Copy link
Copy Markdown
MemberAuthor

Windows test failure seems unrelated.

@nikic

Copy link
Copy Markdown
Member

Merged as 6d6d954, thanks!

@nikicnikic closed this Jul 23, 2019
@Girgias
Girgias deleted the e-strict-cleanup branch July 23, 2019 11:43
Sign up for freeto 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.

3 participants

@Girgias@nikic@Tobion