Show the integer size in phpinfo() #12201
Conversation
|
Please no new features in RC whatever small are. Also this broke some tests so you need to fix them too. So master only for this. |
|
I don't see as such a big deal as well as honestly who is even using 32bit nowadays... :) |
Target branch adjusted.
phpinfo() is tested? 🙈 Now fixed.
See the discussion in the referenced issue for a reasoning. Also 32-bit ARM is not too uncommon and if PHP supports 32-bit systems and the behavior changes depending on the architecture, it should probably be exposed in phpinfo(). |
|
Yeah I'm fine with exposing that. Just don't see it that urgent so we would need to do exception for 8.3 as it's the first time someone is asking for this even though 32bit was much more common in past... |
Exactly, when 32-bit installations were more common, requiring 64-bit installation might not have been feasible yet for broad compatibility, but it is now, given that 32-bit installations are rare. But of course users that actually use them should be able to easily check. |
| @@ -29,6 +29,7 @@ PHP Extension => %d | |||
| Zend Extension => %d | |||
| Zend Extension Build => API%s | |||
| PHP Extension Build => API%s | |||
| Integer Size => %d-bit | |||
There was a problem hiding this comment.
Would it make sense to maybe call it PHP Integer Size? It could be a bit confusing for people familiar with C.
There was a problem hiding this comment.
And also should the result be more 32 bits or 64 bits instead? The 64-bit is more for architucture identification.
There was a problem hiding this comment.
Both changed. I wasn't quite sure about "32 bits" vs "32-bit" myself.
a0c2590 to
78eab66
Compare
|
Thanks, did the actual rebase+squash onto master from PHP-8.3, added NEWS and merged. |
I've opted to place it at the top of the list of "features", but I'm open to bike-shedding the location (directly above IPv6 support?).
I'm also targeting PHP 8.3, as this is a trivial change to a function that does not guarantee stable output in the first place. But pinging the RMs @ericmann, @bukka, @adoy in case they disagree and I'm happy to adjust the target branch.
Resolves GH-12188