feat(rust): PropertyGet quotation pattern#4800
Closed
Thorium wants to merge 3 commits into
Closed
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Makes the
PropertyGetquotation pattern work on the Rust target, and unifies theSystem.Reflection.PropertyInfocarrier so reflection and quotations share one representation.Stacked on #4787 (
feat(rust): reflection) — the diff to review is the last commit only.The problem
F#'s pattern is
PropertyGet(instance: Expr option, propInfo: PropertyInfo, indexerArgs: Expr list)— three slots — and the pattern mapping is shared by every target (Replacements.Util.fs,"PropertyGetPattern"->isFieldGet). The Rust runtime'sisFieldGetreturned only two slots and did not normalise the no-instance sentinel, so aPropertyGetpattern could not match at all:Fixing the arity alone was not enough: the
propInfoslot has to be whateverSystem.Reflection.PropertyInfoerases to, and that type was erased to the nativeReflection_::RecordFieldInfo, which a quotation cannot build (it carries a field-getter closure; a quotation only knows the property name).The change
PropertyInfonow erases to a small F#-declared carrier,FSharpPropertyInfo = { Name: string }, mirroring howUnionCaseInfoandMethodInfoalready erase toFSharpUnionCaseInfo/FSharpMethodInfoinQuotationTypes.fs.PropertyInfowas the odd one out in using a native carrier.One carrier now serves both reflection (
FSharpType.GetRecordFields) and quotations (PropertyGet'spropInfo), as in .NET where both yield the samePropertyInfo.The carrier holds only the name, for two reasons:
Hashable, PartialEq, PartialOrd, which a closure field would break.The field getter is instead resolved from the reflection registry by name when a value is read, which is also what JS/TS does —
getValue(pi, v) => v[pi[0]].RecordFieldInfostays as the registry's internal representation.Testing
./build.sh test rust→ 2550 passed, 0 failed (4 new tests):Option.ValueandList.Headdeconstruct asPropertyGet(Some _, _, [])PropertyGet(None, _, []), proving the"novalue"no-instance sentinel surfaces asNonerather thanSome sentinelNodepi.Namereturns"Value", i.e. the samePropertyInfomember access already used forFSharpType.GetRecordFieldsresultsThe 10 existing record-reflection tests still pass, covering the carrier swap.
Also verified:
./build.sh test rust --threaded→ 2565 passed;--no_stdbuilds clean.🤖 Generated with Claude Code