Skip to content

Add missing scope merge in do-while#1849

Closed
herndlm wants to merge 1 commit intophpstan:1.8.xfrom
herndlm:do-while-scope-merge
Closed

Add missing scope merge in do-while#1849
herndlm wants to merge 1 commit intophpstan:1.8.xfrom
herndlm:do-while-scope-merge

Conversation

@herndlm
Copy link
Copy Markdown
Contributor

@herndlm herndlm commented Oct 14, 2022

Adds the scope merge that all other loops have as well. Fixes the first failure mentioned in #1847 (comment)

I didn't understand it fully though tbh

@herndlm herndlm marked this pull request as ready for review October 15, 2022 00:06
@ondrejmirtes
Copy link
Copy Markdown
Member

Without a test this doesn't tell us anything 😊

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Oct 15, 2022

Yeah I didn't succeed in creating one unfortunately. But in the linked PR I added the same thing and there it's needed. This is a hard one to argue about :/

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Oct 15, 2022

I think the problem was that the never only happens inside the loop iteration and if I try to dump the type I only see the merged scopes where the never is eliminated. But in the other PR it is problematic because it is used via array_shift/pop and actually widens the type via the new methods. At least that's what I think so far.
But I agree, here without a test this is useless..

@ondrejmirtes
Copy link
Copy Markdown
Member

So it's linked to the other PR. If you can't make its tests pass without this, it belongs there.

@herndlm herndlm closed this Oct 15, 2022
@herndlm herndlm deleted the do-while-scope-merge branch October 15, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants