Repository navigation
Conversation
|
This turned out more complex than I had anticipated. Conceptually this mimics a part of type inference again, and seems fragile. There must be a nicer way to do this. |
Track numeric operand types before range inference with a lightweight worklist. Coercions and symbolic constraints must not reuse bounds that apply only to integer inputs. Guard casts, arithmetic, increment/decrement, and assignments through typed references, then run full type inference using the valid ranges. Fixes phpGH-24088
dcf148c to
b0f610f
Compare
|
A comparison bound says nothing about the int value of a bool, string or array ( Also fixed assignment through a $GLOBALS typed reference in b0f610f. |
|
I'm not reviewing ai generated code, it tends to bolt things on top rather than doing an integrated fix, and it definitely feels like this is the case here |
|
There is a fair bit of human slop (mine) here too (I dunno if that makes it worse 🥹) As a general note, in case you or someone else wants to pursue different path. This change gives smallest performance impact (although it is still 1-2% in micro bench) other approaches are fair more painful 5% impact, although they may "look cleaner or general" (had LLM draft variants for review), but I didn't like them mostly for performance reasons, and actually scope started to seem more dangerous even to me. |
|
The globals issue seems SCCP related rather than type inference related. It reminds me of a patch (in an open PR) I have for typed references/properties. |
I really wish to know how much is automatic and how much is done by hand. You open PRs at record speeds, sometimes in such a short span that it must be automatically opened. Obviously your patches are largely automated. On first thought on this patch: I think it's cleaner to rerun range inference+type inference until a fixpoint is reached. Dropping the ranges on constrained inputs after type inference based on a loop over the phi nodes. Sure there's a hit, but I expect this to be rare. |
|
Not sure if it is actually SCCP related, might be worth a second look. |
Integer ranges from comparisons only constrain integer inputs. Track numeric operand types with a lightweight worklist before range inference so casts, arithmetic, symbolic comparisons, and typed-reference assignments cannot reuse incompatible bounds. This prevents values such as 5.5 and true from being folded to 6 while retaining a single full type-inference pass. Some symbolic loop bounds derived from arithmetic remain less precise because the preliminary type information cannot use range-dependent integer certainty. Fixes #24088.