Accept a constant expression as an array bound - #12
Open
partouf wants to merge 1 commit into
Open
Conversation
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>
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.
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- Expressionidiom would meet the ']' of the commonarray[TIndex] of Byteand 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,
*,-andshl, 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).