Repository navigation
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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. Without the parser change it fails with the original
'SquareClose' expected found '+', so it guards the actual bug.
Suite: 43 tests, 42 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) <noreply@anthropic.com>
TSyntaxNode.SetAttribute pointed its entry pointer at a slot only when it added the key. For a key the node already carried the pointer was left uninitialised and the value was written through it anyway; dcc32 says so with W1036 on AttributeEntry. Any directive stored twice reaches it: procedure x(a: Integer); stdcall; stdcall; external 'a.dll' index 93; is legal Delphi, stores anCallingConvention twice and ends in an access violation, or in a silent write to whatever address the stack held. SetAttribute now looks the existing entry up with TryGetAttributeEntry and overwrites its value, and adds an entry only for a new key. An empty value removes the attribute and returns, instead of falling through to the same uninitialised pointer after RemoveAttribute. The last value stored wins, so `stdcall; cdecl;` records cdecl.
SetAttribute with an empty value goes through RemoveAttribute, which did not remove anything. It computed the byte offset of the entry past the one being removed and then moved the entries one slot further along rather than back, so the removed entry stayed, the one before the last was overwritten, and the array kept its length. Only the key's bit in FAttributesInUse was cleared, which hid the stale entry from HasAttribute but not from Attributes, so the writers and the binary serializer still saw it. The Move also copied the string values bit for bit, leaving two entries owning one reference. RemoveAttribute now shifts the entries after the removed one back by assignment, which keeps the string reference counts right, and shortens the array by one.
TPasSyntaxTreeBuilder.Run turns an ESyntaxError into an ESyntaxTreeException, whose constructor takes the line first and the column second. It passed E.PosXY.X, E.PosXY.Y, but TmwBasePasLex.GetPosXY puts the column in X and the line in Y, so the two came out swapped. The EParserException path next to it was right; ParserMessage already passes Y, X. ESyntaxError is what ExpectedFatal raises, which is how an unexpected end of file is reported. A unit that stops after the `begin` of a routine on line 5 unit Broken; interface implementation procedure Run; begin was reported at line 6, col 5 instead of line 5, col 6.
Overwrite an existing attribute instead of writing through an uninitialised pointer
Report a syntax error at its line, not its column
An FPC built with dotted unit names (FPC_DOTTEDUNITS, as in FPC trunk's
namespaced RTL) ships System.Generics.Collections and no unit called
Generics.Collections, so DelphiAST does not compile against it as-is.
Reaching the undotted name through a unit scope (-FNSystem) is not enough
either: a generic specialised in two units that each found
Generics.Collections that way comes out as two incompatible types, e.g.
DelphiAST.pas(726,67) Error: Incompatible type for arg no. 2:
Got "TList<DelphiAST.Classes.TSyntaxNode>",
expected "TLIST<DelphiAST.Classes.TSyntaxNode>"
Follow the convention the dotted RTL itself uses and name the dotted unit
under {$IFDEF FPC_DOTTEDUNITS}. Nothing defines that for user code, so a
dotted build passes -dFPC_DOTTEDUNITS; Delphi and an undotted FPC see
exactly the uses clauses they saw before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
No description provided.