Skip to content

Promote warnings to errors in array_push() - #4585

Closed
Girgias wants to merge 1 commit into
php:masterfrom
Girgias:push-array-warnings2error
Closed

Promote warnings to errors in array_push()#4585
Girgias wants to merge 1 commit into
php:masterfrom
Girgias:push-array-warnings2error

Conversation

@Girgias

Copy link
Copy Markdown
Member

Split from #4566

Comment threadext/standard/array.c 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.

So, this is actually a pretty generic warning, in particular also used by $foo[] = $x, see

zend_error(E_WARNING, "Cannot add element to the array as the next element is already occupied");
. I think whatever we do here, we should have a consistent behavior between $foo[] = $x and array_push($foo, $x).

@Girgias
Girgiasforce-pushed the push-array-warnings2error branch from 59b3ab9 to 8f780abCompareAugust 26, 2019 15:53
@Girgias

Copy link
Copy Markdown
MemberAuthor

Appveyor failure seems unrelated.

Comment threadext/standard/array.c Outdated
@Girgias
Girgiasforce-pushed the push-array-warnings2error branch from 2a753b9 to 7680d5bCompareAugust 29, 2019 21:38
@Girgias

Copy link
Copy Markdown
MemberAuthor

This probably needs to wait the result of the Reclassifying Engine Warnings RFC.

@cmb69

Copy link
Copy Markdown
Member

That RFC has been accepted (this issue should trigger an Error exception), so we can proceed with this PR. :)

@nikic

nikic commented Oct 2, 2019

Copy link
Copy Markdown
Member

Merged as 1ca4ab0.

@nikicnikic closed this Oct 2, 2019
@Girgias
Girgias deleted the push-array-warnings2error branch November 20, 2019 19:25
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.

8 participants

@Girgias@cmb69@nikic@SjonHortensius@theodorejb@krakjoe@kocsismate@carusogabriel