Skip to content

Make bcround pure#5499

Merged
staabm merged 4 commits intophpstan:2.1.xfrom
still-dreaming-1:patch-1
Apr 22, 2026
Merged

Make bcround pure#5499
staabm merged 4 commits intophpstan:2.1.xfrom
still-dreaming-1:patch-1

Conversation

@still-dreaming-1
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

You have to run ./bin/generate-function-metadata.php in order to update also resources/functionMetadata

@still-dreaming-1
Copy link
Copy Markdown
Contributor Author

You have to run ./bin/generate-function-metadata.php in order to update also resources/functionMetadata

Ok, thanks. I will try it on my machine. My initial attempt was an edit straight from GitHub.

@VincentLanglet
Copy link
Copy Markdown
Contributor

You have to run ./bin/generate-function-metadata.php in order to update also resources/functionMetadata

Ok, thanks. I will try it on my machine. My initial attempt was an edit straight from GitHub.

Editing manually the second file manually might work too.

@ondrejmirtes
Copy link
Copy Markdown
Member

Never manually edit a generated file 😊

@still-dreaming-1
Copy link
Copy Markdown
Contributor Author

still-dreaming-1 commented Apr 20, 2026

Never manually edit a generated file 😊

Good point. I already did, and messed it up, and then hopefully fixed it...

@still-dreaming-1
Copy link
Copy Markdown
Contributor Author

I guess if you think it's good enough, you can merge it. Otherwise if you close this without merging I can start over and try it locally on my system. If I do it locally on my system, do I need to install php 8.2? I currently have 8.5.5.

@VincentLanglet
Copy link
Copy Markdown
Contributor

I ran it

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 21, 2026

@still-dreaming-1 could you give us an example how you find out that bcround is not pure and what impact you expect from this PR?

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 21, 2026

(I am asking, because the PR as is does e.g. not resolve https://phpstan.org/r/abbea322-e316-4be5-8f15-e2d9f6db4a70)

@still-dreaming-1
Copy link
Copy Markdown
Contributor Author

still-dreaming-1 commented Apr 21, 2026

(I am asking, because the PR as is does e.g. not resolve https://phpstan.org/r/abbea322-e316-4be5-8f15-e2d9f6db4a70)

That is a good example of what I'm wanting to be able to do without that issue. I would have used @phpstan-pure unless @pure does the same thing (I guess it does or else the issue would not appear...). The next time I do a PR I will try to do it locally on my system and try to test if it resolves my issue or not.

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 22, 2026

I would have used @phpstan-pure unless @pure does the same thing (I guess it does or else the issue would not appear...).

yes. its the same thing.


I just realized the error in my testing.

bcround is only available in PHP 8.4+ and I tested on 8.3.

the PR is fine, thanks

@staabm staabm merged commit bf41f98 into phpstan:2.1.x Apr 22, 2026
718 of 721 checks passed
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.

4 participants