-
Notifications
You must be signed in to change notification settings - Fork 19
Preserve pointer identity for float NaNs #1144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
kyokuping
wants to merge
2
commits into
youknowone:main
Choose a base branch
from
kyokuping:nan-pointer-identity
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
119 changes: 119 additions & 0 deletions
119
pyre/extra_tests/parity_tests/float_subclass_unboxed_storage.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| # CPython-suite gap: the suite checks float-subclass arithmetic and coercion, | ||
| # not whether a subclass instance survives storage in a container. | ||
| # parity-tests reason: pyre's raw-f64 storage reboxes on read as an exact float, | ||
| # which only `type()` / `is` sees. | ||
|
|
||
| # `FloatListStrategy.is_correct_type`, `AbstractAttribute._pick_unbox_type` and | ||
| # `makespecialisedtuple2` are all strict `type(w) is W_FloatObject`, so a float | ||
| # SUBCLASS instance must keep its box. pyre gives a subclass the same builtin | ||
| # `ob_type` and separates it only by `w_class`, so every JIT fast path that | ||
| # unboxes into raw-f64 storage needs a `w_class` guard, not just a class guard: | ||
| # a class guard alone lets a subclass reuse a trace recorded for exact floats. | ||
|
|
||
| f = 1.5 | ||
|
|
||
|
|
||
| class F(float): | ||
| pass | ||
|
|
||
|
|
||
| s = F(2.5) | ||
|
|
||
| assert type(s) is F | ||
| assert s == 2.5 | ||
| assert type(f) is float | ||
|
|
||
| # --- arity-2 tuple: `makespecialisedtuple2` / Cls_ff ----------------------- | ||
| t = (s, s) | ||
| assert t[0] is s and t[1] is s | ||
|
|
||
| # One subclass slot is enough — both slots are stored raw. | ||
| mixed = (f, s) | ||
| assert mixed[0] is f and mixed[1] is s | ||
|
|
||
| # --- instance attribute: mapdict `UnboxedPlainAttribute` ------------------- | ||
| class C: | ||
| pass | ||
|
|
||
|
|
||
| c = C() | ||
| c.x = s | ||
| assert c.x is s and c.__dict__["x"] is s | ||
|
|
||
| # An existing unboxed float slot must convert back to boxed storage. | ||
| c2 = C() | ||
| c2.y = f | ||
| c2.y = s | ||
| assert c2.y is s | ||
|
|
||
| # --- list: FloatListStrategy / IntOrFloatListStrategy ---------------------- | ||
| lst = [s] | ||
| assert lst[0] is s | ||
|
|
||
| lst2 = [1.0, 2.0] | ||
| lst2.append(s) | ||
| assert lst2[2] is s and type(lst2[0]) is float | ||
|
|
||
| lst3 = [1, 2.0] | ||
| lst3.append(s) | ||
| assert lst3[2] is s | ||
|
|
||
| lst4 = [1.0, 2.0] | ||
| lst4[0] = s | ||
| assert lst4[0] is s | ||
|
|
||
| lst5 = [] | ||
| lst5.append(s) | ||
| assert lst5[0] is s | ||
|
|
||
|
|
||
| # Hand each finite-float trace a subclass instance on its final iteration. | ||
| # A guard failure exits the whole loop iteration, so each direct store needs | ||
| # its own loop. | ||
| def warm_attr(rounds): | ||
| obj = C() | ||
| obj.x = 0.5 | ||
| for i in range(rounds): | ||
| v = s if i == rounds - 1 else i * 0.5 | ||
| obj.x = v | ||
| return obj.x is s | ||
|
|
||
|
|
||
| def warm_setitem(rounds): | ||
| lst = [0.5] | ||
| for i in range(rounds): | ||
| v = s if i == rounds - 1 else i * 0.5 | ||
| lst[0] = v | ||
| return lst[0] is s | ||
|
|
||
|
|
||
| def warm_newlist(rounds): | ||
| for i in range(rounds): | ||
| v = s if i == rounds - 1 else i * 0.5 | ||
| got = [v] | ||
| return got[0] is s | ||
|
|
||
|
|
||
| def warm_append_empty(rounds): | ||
| for i in range(rounds): | ||
| v = s if i == rounds - 1 else i * 0.5 | ||
| got = [] | ||
| got.append(v) | ||
| return got[0] is s | ||
|
|
||
|
|
||
| def warm_append_float(rounds): | ||
| for i in range(rounds): | ||
| v = s if i == rounds - 1 else i * 0.5 | ||
| got = [0.5] | ||
| got.append(v) | ||
| return got[1] is s | ||
|
|
||
|
|
||
| assert warm_attr(3000) | ||
| assert warm_setitem(3000) | ||
| assert warm_newlist(3000) | ||
| assert warm_append_empty(3000) | ||
| assert warm_append_float(3000) | ||
|
|
||
| print("OK") |
106 changes: 106 additions & 0 deletions
106
pyre/extra_tests/parity_tests/nan_unboxed_storage_identity.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| # CPython-suite gap: the suite checks NaN value semantics, not identity through containers. | ||
| # parity-tests reason: pyre's raw-f64 storage reboxes on read, which only `is` / `id()` sees. | ||
|
|
||
| # Python 3.14 gives NaNs pointer identity. Raw-f64 list, tuple, and mapdict | ||
| # storage must therefore reject them instead of reboxing on read. These checks | ||
| # cover the interpreter and JIT paths; finite floats must remain eligible. | ||
|
|
||
| n = float("nan") | ||
| m = float("nan") | ||
| f = 1.5 | ||
|
|
||
| assert n is not m | ||
| assert id(n) != id(m) | ||
| assert n is n and n != n | ||
|
|
||
| # --- arity-2 tuple: `makespecialisedtuple2` / Cls_ff ----------------------- | ||
| t = (n, n) | ||
| assert t[0] is n and t[1] is n | ||
|
|
||
| # One NaN makes the whole pair ineligible for `Cls_ff`. | ||
| mixed = (f, n) | ||
| assert mixed[0] is f and mixed[1] is n | ||
|
|
||
| # --- instance attribute: mapdict `UnboxedPlainAttribute` ------------------- | ||
| class C: | ||
| pass | ||
|
|
||
|
|
||
| c = C() | ||
| c.x = n | ||
| assert c.x is n and c.__dict__["x"] is n | ||
|
|
||
| # An existing float slot must convert back to boxed storage. | ||
| c2 = C() | ||
| c2.y = f | ||
| c2.y = n | ||
| assert c2.y is n | ||
|
|
||
| # --- list: FloatListStrategy / IntOrFloatListStrategy ---------------------- | ||
| lst = [n] | ||
| assert lst[0] is n | ||
|
|
||
| lst2 = [1.0, 2.0] | ||
| lst2.append(n) | ||
| assert lst2[2] is n | ||
|
|
||
| lst3 = [1, 2.0] | ||
| lst3.append(n) | ||
| assert lst3[2] is n | ||
|
|
||
| # `list.index`/`count` compare by value; a NaN is found only via the identity | ||
| # shortcut `==` gets before the value compare. | ||
| assert lst.index(n) == 0 | ||
| assert lst.count(n) == 1 | ||
| assert n in lst | ||
|
|
||
|
|
||
| # Hand each finite-float trace a NaN on its final iteration. A guard failure | ||
| # exits the whole loop iteration, so each direct store needs its own loop. | ||
| def warm_attr(rounds): | ||
| obj = C() | ||
| obj.x = 0.5 | ||
| for i in range(rounds): | ||
| v = n if i == rounds - 1 else i * 0.5 | ||
| obj.x = v | ||
| return obj.x is n | ||
|
|
||
|
|
||
| def warm_setitem(rounds): | ||
| lst = [0.5] | ||
| for i in range(rounds): | ||
| v = n if i == rounds - 1 else i * 0.5 | ||
| lst[0] = v | ||
| return lst[0] is n | ||
|
|
||
|
|
||
| def warm_newlist(rounds): | ||
| for i in range(rounds): | ||
| v = n if i == rounds - 1 else i * 0.5 | ||
| got = [v] | ||
| return got[0] is n | ||
|
|
||
|
|
||
| def warm_append_empty(rounds): | ||
| for i in range(rounds): | ||
| v = n if i == rounds - 1 else i * 0.5 | ||
| got = [] | ||
| got.append(v) | ||
| return got[0] is n | ||
|
|
||
|
|
||
| def warm_append_float(rounds): | ||
| for i in range(rounds): | ||
| v = n if i == rounds - 1 else i * 0.5 | ||
| got = [0.5] | ||
| got.append(v) | ||
| return got[1] is n | ||
|
|
||
|
|
||
| assert warm_attr(3000) | ||
| assert warm_setitem(3000) | ||
| assert warm_newlist(3000) | ||
| assert warm_append_empty(3000) | ||
| assert warm_append_float(3000) | ||
|
|
||
| print("OK") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once NaNs fall back to pointer identity here, every path that erases the original
W_FloatObjecthas to reject them, not justFloatListStrategy.makespecialisedtuple2still buildsW_SpecialisedTupleObject_fffor exact NaNs andmapdictstill picksUnboxType::Float; both store only the rawf64and rebox on read, so cases liken = float('nan'); t = (n, n); t[0] is nor a NaN instance attribute now become false / get freshid()values even though Python attribute and tuple storage should retain the original object. Please apply the same NaN exclusion to those unboxed float paths before switchingis_w/idto address identity.Useful? React with 👍 / 👎.