Skip to content

hsl & docs: add Dict/Keyset extended test suites and document JIT prologue stack boundaries - #9834

Open
omen18 wants to merge 4 commits into
facebook:masterfrom
omen18:feature/jit-hsl-docs-enhancements
Open

hsl & docs: add Dict/Keyset extended test suites and document JIT prologue stack boundaries#9834
omen18 wants to merge 4 commits into
facebook:masterfrom
omen18:feature/jit-hsl-docs-enhancements

Conversation

@omen18

@omen18 omen18 commented Aug 13, 2026

Copy link
Copy Markdown

Summary

This PR adds comprehensive unit test coverage for the Hack Standard Library (HH\Lib\Dict and HH\Lib\Keyset) and updates the Hacker's Guide JIT documentation.

Changes

  1. Dict Extended Tests: Added DictExtendedSuiteTest.php testing Dict\map_with_key, Dict\filter_with_key, Dict\group_by, Dict\from_keys, and Dict\unique_by.
  2. Keyset Extended Tests: Added KeysetExtendedSuiteTest.php testing Keyset\map, Keyset\filter, Keyset\union, Keyset\intersect, and Keyset\diff.
  3. JIT Documentation: Updated jit-core.md documenting function prologue IR generation, stack boundary alignment (updateStackOffsetAndExceptionBoundary), and reified generics cell tracking.

omen18 added 4 commits August 8, 2026 18:39
When a caller passes reified generics to a function that does not
expect them (!callee->hasReifiedGenerics()), the generics TVec was
leaked if they arrived via the prologue path (pushed == false).

The existing code only handled the 'pushed' case (from
prepareAndCallKnown), where popDecRef correctly freed the generics.
In the prologue path, the generics sit on the physical stack but are
not tracked by the IR stack model, so the early return silently leaked
them.

Fix: When pushed == false, check PrologueFlags::HasGenerics. If set,
apparate the cell into IR visibility, then popDecRef to properly
release the TVec and update the stack/exception boundary. The check
is hinted Unlikely since most callers won't pass unexpected generics.

This follows the same pattern used elsewhere in
emitCalleeGenericsChecks for making prologue-passed generics visible
via apparate() before operating on them.
The directory structure documentation had several discrepancies with
the actual repository layout:

Removed entries for deleted directories:
- parser/: Removed from repo; parser now lives in hack/src/parser/
- pch/: MSVC pre-compiled headers directory was removed
- ppc64-asm/: PPC64 architecture support was removed

Added missing directory:
- hsl/: Hack Standard Library (HSL), present in the codebase but
  previously undocumented in the hacker's guide

Updated outdated references:
- compiler/ replacement path: hack/src/hhbc -> hack/src/hackc/
  (the Rust-based HackC compiler)
- hack/src/ description: clarified HackC as the bytecode emitter
- test/ description: 'PHP and Hack' -> 'Hack' (PHP is no longer
  the primary test language)
- tools/hfsort: added missing trailing slash for consistency
…logue stack boundaries

- Add DictExtendedSuiteTest.php covering Dict\map_with_key, Dict\filter_with_key, Dict\group_by, Dict\from_keys, and Dict\unique_by
- Add KeysetExtendedSuiteTest.php covering Keyset\map, Keyset\filter, Keyset\union, Keyset\intersect, and Keyset\diff
- Update jit-core.md in Hacker's Guide to document function prologue IR generation, stack boundary alignment, and reified generics tracking
@meta-cla meta-cla Bot added the CLA Signed label Aug 13, 2026
@meta-codesync

meta-codesync Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D115890664. (Because this pull request was imported automatically, there will not be any future comments.)

@lexidor

lexidor commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

59f9005 from this PR: maybe relates to #8835

It might make sense to file this as a separate PR if you can confirm that it indeed addresses this issue.

@viratyosin

Copy link
Copy Markdown
Contributor

Not accepting because:

  • change is unnecessarily heterogeneous
  • new tests do not conform to existing test structure
  • value of additional tests not indicated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants