From 378d9042ce5827a37032178466aacb4a5f77ac9b Mon Sep 17 00:00:00 2001 From: Patrick Quist Date: Wed, 19 Aug 2026 12:49:27 +0200 Subject: [PATCH 1/3] Give ntConstant an end position A constant declaration produced a node with no end position at all, so a consumer working in line ranges could not tell how far the declaration reached. That silently truncates any constant whose value spans lines: const Banner = 'first part ' + 'second part'; reported only the first line, and a tool slicing that range dropped the continuation. ntConstant was built with FStack.Push, so it was a plain TSyntaxNode with nowhere to record an end. Two changes are needed, because the node the caller finally sees is not the node that was parsed: ConstantDeclaration now pushes a compound node and records its end, the same way TypeDeclaration already does. This gives the intermediate ConstList an accurate extent. ConstSection rebuilds each constant from that ConstList, so it also has to push a compound node and inherit the end from it. The start still comes from the name, which is what a caller looking for the constant expects; only the end is new. TCompoundSyntaxNode.AssignEndPositionFrom is the counterpart to the existing AssignPositionFrom, for exactly this rebuild case. Single-line constants keep ending on their own line - the new test asserts both directions, since an end that ran on to the next declaration would be no more useful than one that stopped short. Verified against the existing suite: 42 passing, with the one pre-existing Serialization.BinaryRoundTrip failure unchanged (line_seq holds a pointer value that does not survive a round trip, unrelated to this change). Co-Authored-By: Claude Opus 5 (1M context) --- Source/DelphiAST.Classes.pas | 8 +++++++ Source/DelphiAST.pas | 9 ++++++-- Test/UnitTests/DelphiAST.Tests.pas | 34 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/Source/DelphiAST.Classes.pas b/Source/DelphiAST.Classes.pas index 10b785b..8752255 100644 --- a/Source/DelphiAST.Classes.pas +++ b/Source/DelphiAST.Classes.pas @@ -104,6 +104,8 @@ TCompoundSyntaxNode = class(TSyntaxNode) FEndLine: Integer; public function Clone: TSyntaxNode; override; + //counterpart to AssignPositionFrom, for a node rebuilt from a parsed one + procedure AssignEndPositionFrom(const Node: TCompoundSyntaxNode); property EndCol: Integer read FEndCol write FEndCol; property EndLine: Integer read FEndLine write FEndLine; @@ -629,6 +631,12 @@ procedure TSyntaxNode.AssignPositionFrom(const Node: TSyntaxNode); { TCompoundSyntaxNode } +procedure TCompoundSyntaxNode.AssignEndPositionFrom(const Node: TCompoundSyntaxNode); +begin + FEndCol := Node.EndCol; + FEndLine := Node.EndLine; +end; + function TCompoundSyntaxNode.Clone: TSyntaxNode; begin Result := inherited; diff --git a/Source/DelphiAST.pas b/Source/DelphiAST.pas index 7a1dcb5..d25d70f 100644 --- a/Source/DelphiAST.pas +++ b/Source/DelphiAST.pas @@ -1195,9 +1195,11 @@ procedure TPasSyntaxTreeBuilder.CompoundStatement; procedure TPasSyntaxTreeBuilder.ConstantDeclaration; begin - FStack.Push(ntConstant); + //compound: a constant's value can span lines, so record where it ends. as TypeDeclaration. + FStack.PushCompoundSyntaxNode(ntConstant); try inherited; + SetCurrentCompoundNodesEndPosition; finally FStack.Pop; end; @@ -1329,9 +1331,12 @@ procedure TPasSyntaxTreeBuilder.ConstSection; if Constant.Typ <> ntName then Continue; - Temp := FStack.Push(ConstList.Typ); + //compound: start from the name, end from the ConstList that measured the value. + Temp := FStack.PushCompoundSyntaxNode(ConstList.Typ); try Temp.AssignPositionFrom(Constant); + if ConstList is TCompoundSyntaxNode then + TCompoundSyntaxNode(Temp).AssignEndPositionFrom(TCompoundSyntaxNode(ConstList)); FStack.AddChild(Constant.Clone); if Assigned(TypeInfo) then diff --git a/Test/UnitTests/DelphiAST.Tests.pas b/Test/UnitTests/DelphiAST.Tests.pas index 6d15948..96cf3c6 100644 --- a/Test/UnitTests/DelphiAST.Tests.pas +++ b/Test/UnitTests/DelphiAST.Tests.pas @@ -177,6 +177,39 @@ procedure TestSourcePositions; end; end; +procedure TestConstantEndPosition; +var + Root, ConstsNode, Node, Spanning, Single: TSyntaxNode; +begin + Root := ParseSource('unit Consts;' + sLineBreak + 'interface' + sLineBreak + 'const' + + sLineBreak + ' Spanning = ''first part '' +' + sLineBreak + ' ''second part'';' + + sLineBreak + ' Single = 42;' + sLineBreak + 'implementation' + sLineBreak + 'end.'); + try + ConstsNode := FindDescendant(Root, ntConstants); + AssertNotNil(ConstsNode, 'No const section was produced'); + Spanning := nil; + Single := nil; + for Node in ConstsNode.ChildNodes do + if Node.Typ = ntConstant then + if Node.Line = 4 then + Spanning := Node + else if Node.Line = 6 then + Single := Node; + + AssertNotNil(Spanning, 'No constant starting on line 4'); + AssertTrue(Spanning is TCompoundSyntaxNode, + 'Constant node must be compound so it can carry an end position'); + AssertEquals(5, TCompoundSyntaxNode(Spanning).EndLine, + 'A multi-line constant must reach the last line of its value'); + + AssertNotNil(Single, 'No constant starting on line 6'); + AssertEquals(6, TCompoundSyntaxNode(Single).EndLine, + 'A single-line constant must end on its own line, not run on to what follows'); + finally + Root.Free; + end; +end; + procedure TestInvalidSyntax; var Root: TSyntaxNode; @@ -229,6 +262,7 @@ procedure RunAllTests; RunTest('AST.GenericRecordAndProperty', TestGenericRecordAndProperty); RunTest('Writer.LiteralsUnicodeAndXmlEscaping', TestLiteralsAndUnicode); RunTest('AST.SourcePositions', TestSourcePositions); + RunTest('AST.ConstantEndPosition', TestConstantEndPosition); RunTest('Parser.InvalidSyntax', TestInvalidSyntax); {$IFNDEF FPC} RunTest('Serialization.BinaryRoundTrip', TestBinarySerializationRoundTrip); From e4258bcd7f387e7895c1f56b80c2fb1d928adca6 Mon Sep 17 00:00:00 2001 From: Patrick Quist Date: Wed, 19 Aug 2026 14:11:44 +0200 Subject: [PATCH 2/3] Give ntVariable an end position Same gap as the previous commit, same shape of fix. A variable declaration produced a node positioned at its name with no end at all, so a declaration whose type or initialiser spans lines reported only its first line: var Grid: array[0..1] of Integer; VarDeclaration pushed ntVariables with FStack.Push, so the VarList that RearrangeVarSection rebuilds each variable from had no extent to pass on. Both now push compound nodes, and the rebuilt ntVariable inherits the end via AssignEndPositionFrom - the second caller for the helper added in the previous commit, which is the pattern it exists for. The compound ntVariables nodes are the throwaway VarSect children that VarSection frees, so the output footprint matches the constant change exactly: VARIABLE gains begin/end, and the VARIABLES section node that reaches the tree is untouched. Names sharing one line (`A, B: Integer;`) each get the declaration's extent, which is the whole declaration they share. Co-Authored-By: Claude Opus 5 (1M context) --- Source/DelphiAST.pas | 9 +++++++-- Test/UnitTests/DelphiAST.Tests.pas | 32 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Source/DelphiAST.pas b/Source/DelphiAST.pas index d25d70f..5ff7b5c 100644 --- a/Source/DelphiAST.pas +++ b/Source/DelphiAST.pas @@ -3210,9 +3210,11 @@ procedure TPasSyntaxTreeBuilder.VarAbsolute; procedure TPasSyntaxTreeBuilder.VarDeclaration; begin - FStack.Push(ntVariables); + //compound: a variable's type or initialiser can span lines, so record where it ends. + FStack.PushCompoundSyntaxNode(ntVariables); try inherited; + SetCurrentCompoundNodesEndPosition; finally FStack.Pop; end; @@ -3303,9 +3305,12 @@ procedure TPasSyntaxTreeBuilder.RearrangeVarSection(const VarSect: TSyntaxNode); begin if Variable.Typ <> ntName then Continue; - Temp := FStack.Push(ntVariable); + //compound: start from the name, end from the VarList that measured the declaration. + Temp := FStack.PushCompoundSyntaxNode(ntVariable); try Temp.AssignPositionFrom(Variable); + if VarList is TCompoundSyntaxNode then + TCompoundSyntaxNode(Temp).AssignEndPositionFrom(TCompoundSyntaxNode(VarList)); FStack.AddChild(Variable.Clone); if Assigned(TypeInfo) then FStack.AddChild(TypeInfo.Clone); diff --git a/Test/UnitTests/DelphiAST.Tests.pas b/Test/UnitTests/DelphiAST.Tests.pas index 96cf3c6..c46a2bb 100644 --- a/Test/UnitTests/DelphiAST.Tests.pas +++ b/Test/UnitTests/DelphiAST.Tests.pas @@ -210,6 +210,37 @@ procedure TestConstantEndPosition; end; end; +procedure TestVariableEndPosition; +var + Root, Node, Spanning, Single: TSyntaxNode; +begin + Root := ParseSource('unit Vars;' + sLineBreak + 'interface' + sLineBreak + 'var' + + sLineBreak + ' Spanning: array[0..1] of' + sLineBreak + ' Integer;' + sLineBreak + + ' Single: Integer;' + sLineBreak + 'implementation' + sLineBreak + 'end.'); + try + Spanning := nil; + Single := nil; + for Node in FindDescendant(Root, ntVariables).ChildNodes do + if Node.Typ = ntVariable then + if Node.Line = 4 then + Spanning := Node + else if Node.Line = 6 then + Single := Node; + + AssertNotNil(Spanning, 'No variable starting on line 4'); + AssertTrue(Spanning is TCompoundSyntaxNode, + 'Variable node must be compound so it can carry an end position'); + AssertEquals(5, TCompoundSyntaxNode(Spanning).EndLine, + 'A variable whose declaration spans lines must reach its last line'); + + AssertNotNil(Single, 'No variable starting on line 6'); + AssertEquals(6, TCompoundSyntaxNode(Single).EndLine, + 'A single-line variable must end on its own line, not run on to what follows'); + finally + Root.Free; + end; +end; + procedure TestInvalidSyntax; var Root: TSyntaxNode; @@ -263,6 +294,7 @@ procedure RunAllTests; RunTest('Writer.LiteralsUnicodeAndXmlEscaping', TestLiteralsAndUnicode); RunTest('AST.SourcePositions', TestSourcePositions); RunTest('AST.ConstantEndPosition', TestConstantEndPosition); + RunTest('AST.VariableEndPosition', TestVariableEndPosition); RunTest('Parser.InvalidSyntax', TestInvalidSyntax); {$IFNDEF FPC} RunTest('Serialization.BinaryRoundTrip', TestBinarySerializationRoundTrip); From bb683d0408c1e23738c85ee3acc47b021c7c3928 Mon Sep 17 00:00:00 2001 From: Patrick Quist Date: Mon, 31 Aug 2026 17:17:12 +0200 Subject: [PATCH 3/3] Accept a constant expression as an array bound An array bound may be any constant expression, but OrdinalType only ever accepted a constant or a type name: const mlab = 4; mlog = 12; type TRanges = record iu: array[mlab + 1..mlog] of Integer; end; failed with 'SquareClose' expected found '+'. OrdinalType decided on a single token of lookahead: an identifier followed by anything but '(' or '..' was a type name, so it read the bound as the type `mlab`, returned, and left ArrayBounds looking for the ']' it found a '+' at. The upper bound already worked, which is what makes the gap easy to miss. `array[mlab..mlog + 1]` sends the first bound to ConstantExpression on the '..' lookahead, and OrdinalType's own trailing '..' branch parses the rest as an expression. Only the first bound of a subrange went down the type-name path. The lookahead now also routes the operators SimpleExpression and Term accept to ConstantExpression. Every one of those tokens is a parse error in this position today, so no input that parses now takes a different path: an identifier in an OrdinalType is followed by ']', ',', '..', 'of' or ';', never by an operator. Set types and variant record tag types go through the same procedure and gain the same forms. A single token is enough here and a full ahead-parse would be worse. Coming from the identifier, SimpleType's `AheadParse.NextToken; AheadParse.Simple- Expression` idiom would meet the ']' of the common `array[TIndex] of Byte` and hand it to Factor as a set constructor. Parenthesized bounds are deliberately left alone. `array[(mlab + 1)..mlog]` goes to EnumeratedType, and dcc32 reads it the same way - it reports "Identifier redeclared: 'mlab'" - so the parser already agrees with the compiler. Test/Snippets/arrayboundexpression.pas covers the first bound, both bounds, two dimensions, `*`, `-` and `shl`, and a set of a computed subrange; dcc32 compiles it clean. Suite: 45 tests, 44 passing, with the pre-existing Serialization.BinaryRoundTrip failure unchanged (line_seq holds a pointer value that does not survive a round trip). Co-Authored-By: Claude Opus 5 (1M context) --- Source/SimpleParser/SimpleParser.pas | 10 ++++++++++ Test/Snippets/arrayboundexpression.pas | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 Test/Snippets/arrayboundexpression.pas diff --git a/Source/SimpleParser/SimpleParser.pas b/Source/SimpleParser/SimpleParser.pas index 06caf50..f632596 100644 --- a/Source/SimpleParser/SimpleParser.pas +++ b/Source/SimpleParser/SimpleParser.pas @@ -3648,6 +3648,16 @@ procedure TmwSimplePasPar.OrdinalType; begin ConstantExpression; end; + { An operator after the identifier means the bound is a constant + expression, not a type name: array[mlab + 1..mlog]. Reading it as a + type name consumes the identifier alone and leaves the caller at the + operator, where it can only report an error. These are the operators + SimpleExpression and Term accept. } + ptAnd, ptDiv, ptMinus, ptMod, ptOr, ptPlus, ptShl, ptShr, ptSlash, + ptStar, ptXor: + begin + ConstantExpression; + end; else begin TypeID; diff --git a/Test/Snippets/arrayboundexpression.pas b/Test/Snippets/arrayboundexpression.pas new file mode 100644 index 0000000..1d695d1 --- /dev/null +++ b/Test/Snippets/arrayboundexpression.pas @@ -0,0 +1,22 @@ +unit arrayboundexpression; + +interface + +const + mlab = 4; + mlog = 12; + +type + { An array bound is a constant EXPRESSION, not just a constant or a type name. + OrdinalType decided on one token of lookahead and read `mlab` as a type name, + so the whole unit failed to parse at the `+`. } + TRanges = record + iu: array[mlab + 1..mlog] of Integer; + du: array[mlab * 2..mlog - 1, 0..mlab shl 1] of Byte; + end; + + TSetOfExpression = set of mlab + 1..mlog; + +implementation + +end.