Measured, 2026-08-25
Threading the enum identity onto a field
(bug-p-an-enum-reached-through-a-field-or-index-still-writes-its-ordinal)
required the SAME six edits in both places: reset the capture beside
isArr := False, take it from LastTypeEnumId after each of the three
ParseTypeKind calls, take it from the alias registry in the two named-array
arms, and stamp it after AddUField. The two blocks differ only in indentation
and in the local's name (fEnumId vs fEnumId2 — they are in different
routines, so they cannot even share one).
The arms line up one for one:
| ParseRecordFields | ParseTypeSection's class arm | |
|---|---|---|
| names | fldNOffs/fldNLens loop |
identical |
array[..] of T |
~3238 | ~4861 |
array of T (nested) |
~3254 | ~4879 |
| named dyn alias | ~3275 | ~4896 |
| named fixed alias | ~3292 | ~4910 |
| scalar | ~3318 | ~4934 |
| size/align | if fIsDyn / tyString / tyFixedString / tyRecord |
identical |
AddUField tail |
~3370 | ~4970 |
Why it is a refactor ticket and not a bug
Nothing is wrong TODAY — the enum work put both copies in step. The cost is that
every future field-level attribute is two edits, and the failure mode is silent:
the feature works on records and not on classes (or the reverse), which reads as
a mysterious dialect gap rather than as a missing paste.
devdocs/dev/normalise-dont-special-case.md is exactly this shape, one level up
from the const-vs-variable cases it collects.
Shape of the fix
Lift the block into one ParseFieldDeclInto(ci: Integer; ...) that both callers
invoke, with the visibility/class var/property/method dispatch staying where it
is — the duplication is only the part after Expect(tkColon, ':'). The class arm
carries a couple of extras (published/visibility stamping); those are already
applied AFTER the shared tail, so they survive the lift unchanged.
Gate: the usual make compiler/pascal26 fixedpoint + tools/gate.sh quick. The
lift is a pure code move, so a byte-identical self-host is the strong evidence
here.
Re-derived 2026-09-05 (frankB) — it is THREE, and the third had two live bugs
Re-counted before taking it, because every ticket census re-derived that night was wrong. This one is wrong in the opposite direction from the usual, which is the dangerous one: it UNDERCOUNTS.
$ grep -n 'UFldEnumId\[UFldCount-1\]' compiler/pasparser_decl.inc
4286 -> ParseRecordFields
6258 -> ParseTypeSection
$ grep -n 'AddUField(' compiler/pasparser_decl.inc # by enclosing routine
3320, 3426, 3428, 3432 -> ParseRecordVariantPart <-- the third copy
4279, 4281, 4285 -> ParseRecordFields
6251, 6253, 6257 -> ParseTypeSection
ParseRecordVariantPart parses branch fields with the same grammar and the same
AddUField tail, and it is not in this ticket. Its arms line up against the
other two like this:
| arm | RecordFields | class arm | VariantPart |
|---|---|---|---|
| comma-separated names | yes | yes | yes |
array[..] of T |
yes | yes | yes |
array of T (dynamic) |
yes | yes | no — and correct, FPC refuses refcounted types in a variant part |
| named array alias | yes | yes | NO — silent bug |
| frozen string sizing | yes | yes | yes |
UFldEnumId stamp |
yes | yes | NO — silent bug |
Both gaps were live, and both are fixed (ParseRecordVariantPart now
captures LastTypeEnumId and has a named-fixed-array arm; multi-dim named
aliases are refused with a message rather than mis-sized, matching the inline
spelling that was already refused). Measured on pin v403 against fpc 3.2.2:
pin v403 pxx now / fpc
case 0: (c: TColor) 2 Blue
case 3: (ea: TEArr) 0 2 Red Blue
SizeOf(TVar) 8 20
The size row is the one that matters: with TArr = array[0..3] of Integer in a
branch, ParseTypeKind answered the ELEMENT kind, the field was sized as one
Integer, and v.a[3] := 44 wrote twelve bytes past the end of the record. It
read back correctly, because the read went to the same wrong place — so every
VALUE row on the pin printed a plausible number.
test/test_variant_part_field_arms.pas pins all of it.
This raises the ticket rather than closing it — 45 -> 55
The lift is NOT done; only the two divergences are. The ticket's own argument was that the cost is future edits, "and the failure mode is silent". That is no longer a prediction: it has now happened twice, in a copy the ticket did not know about, and the drift runs both ways — the variant part's own comments record it being right about frozen strings while the other two were wrong ("The variant-part arm a few hundred lines up already tested both kinds; this one and its class sibling did not"), and this pass found it wrong about two things they were right about.
Amendment to the shape of the fix
"Lift the block into one ParseFieldDeclInto" should be three callers, not
two — and the split is not where the ticket puts it. What all three share is
the part between Expect(tkColon, ':') and the size/align computation: the type
spec and what it yields (fTk, fRec, fEnumId, isArr, arrLen, fIsDyn,
fDynDepth, fNDims, the dimension table, fStrCap). What they do NOT share is
the offset bookkeeping: ParseRecordVariantPart resets to branchFieldOff per
branch and tracks variantMax, and its whole reason for existing is that
overlay. Lift the spec parse and the sizing; leave the offsets alone. A lift
that tries to unify the offset half will fight the variant part's overlay model
and is the version of this refactor that goes wrong.
Resolved 2026-09-05 (frankA) -- landed in three commits
ParseFieldDeclInto(ci; var curFieldOff, classAlign; isPackedRecord, inRecordBody, inVariantPart; pub, vis) sits before ParseRecordFields and is
called from all three sites. The amendment above was right about the split: the
routine parses the type spec and does the sizing, and every caller keeps its own
offset bookkeeping -- the variant part passes branchFieldOff and its own
running vMaxAlign, folding that into classAlign after the call, which is the
same answer because the record's alignment can only grow.
Three arms are gated on inRecordBody, because they are about a RECORD and not
about a field: the {$PACKRECORDS} / packed record clamp, the refusal of a
by-value field of the record's own type, and the class-typed-field UClsAlign
lookup. One arm is gated on inVariantPart: a dynamic array is now refused with
a sentence saying WHY (its storage is shared with the other branches, so nothing
can know which finaliser to run). That refusal existed before by ACCIDENT -- the
copy simply had no dynamic-array arm, so array of Integer came out as
expected '[' before 'of'.
What the merge FIXED, each against fpc 3.2.2 as the oracle
| pre-merge pxx | pxx now / fpc | |
|---|---|---|
branch field alignment (SizeOf of two branch records) |
8 8 | 4 1 |
| multi-dimensional array in a branch | mis-sized | 7 7 24 24 |
packed array field in a branch |
refused | 33 33 16 16 |
The alignment row is the one that had pxx contradicting ITSELF: the same field declared in the fixed part got the fixed part's alignment and in a branch did not.
Measured and deliberately NOT fixed
- A class field of
objecttype is aligned to pointer width rather than to its own alignment (TC.InstanceSize24 against fpc's 16). Space, not correctness, and pxx's class header already differs by 6 bytes for unrelated reasons. stringandIUnknownin a variant branch: pxx accepts, fpc refuses. Older than this routine and out of scope; recorded in the test header.
The two things the lift got wrong, both caught by an instrument
- FPC is single-pass and pxx is not.
ParseRecordVariantPart's call sits ABOVE the routine, so the tree self-hosted, passed--tier quick, and broke the bootstrap seed --pasparser_decl.inc(3901,7) Error: Identifier not found "ParseFieldDeclInto", caught by the forwardlint row and the FPC seed canary and by nothing else. Aforward;inpasparser_name.incfixed it, and the rebuild was BYTE-IDENTICAL, which is what a forward declaration should be. - The separating
;is the CALLER's, in all three loops. It was dropped when the block was lifted, so0: (MinValue: Int64; MaxValue: Int64)inlib/rtl/typinfo.pasre-entered the loop sitting on the;. Two quick rows RED, including one whose name has nothing to do with records -- anything linking typinfo.
Tests
test/test_packed_array_field_in_a_variant_branch.pas-- the alignment, multi-dimension andpacked arrayrows above; expected output is fpc's own.test/test_dynarray_in_a_variant_part_refused.pas-- a must-not-compile fixture asserted on the MESSAGE, not on the exit code. fpc refuses it too.test/test_packed_array_field_in_a_record_and_a_class.pas-- the sibling from the previous commit, header updated now that the third kind is covered.
The negative control was real: the pre-merge compiler (2a37001fc113) was
rebuilt and refuses the new test.
Log
- 2026-09-05 — resolved; this names the commit that carried the resolve, which is not always the one that carried the change — commit 2792cec8f.