fix(parser): preserve the original text of COND equations - #12
Open
ThVerg wants to merge 1 commit into
Open
Conversation
Conditional expressions were rewritten on parse and written back in a form no other tool accepts. Two independent mechanisms were responsible. FLOAT and SCALARCONSTANT overlap: the constant's leading digit is optional, so for 1'b0 the lexer matched FLOAT on the 1 and SCALARCONSTANT on the remaining 'b0, and equation() rejoined the pieces with a space, giving B == 1.0 'b0. Separately, equation shared the FLOAT terminal with delay values, and the FLOAT() callback converts every such token to a Python float. That is correct for a delay and destructive for condition text, where 0 and 0.0 are different tokens, so TE == 0 became TE == 0.0 with no scalar constant involved. Conditions now use their own EQNUMBER terminal, so equation never touches FLOAT and no conversion callback fires. LALR mode uses a contextual lexer, so delay positions still lex as FLOAT. SCALARCONSTANT must outrank EQNUMBER or 1'b0 splits again; EQNUMBER covers decimals so 1.5 does not split, and carries an optional sign because FLOAT's -? had been handling negative literals. golden/fixpoint.sdf and golden/spec-example2.sdf are regenerated; the diff is the corruption being undone.
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.
Conditional expressions are rewritten into something no other tool accepts
Condition text is currently passed through Python's
float(), and Verilog scalar constantsare split in half by the lexer:
B==1'b0B == 1.0 'b0B == 1'b0TE == 0TE == 0.0TE == 0A == -1A == -1.0A == -1A==+2A == + 2.0A == +2B==1'b0&&C==1'b1B == 1.0 'b0 && C == 1.0 'b1B == 1'b0 && C == 1'b1A==1.5A == 1.5A == 1.5(unchanged)~EN~ EN~ EN(unchanged)Cause
Two independent mechanisms.
Scalar constants split.
FLOATandSCALARCONSTANToverlap: the constant's leadingdigit is optional (
[01]?), so for1'b0the lexer matchesFLOATon the1first andSCALARCONSTANTon the remaining'b0.equation()then rejoins the pieces with a space.Numbers lose their text.
equationshares theFLOATterminal with delay values, andthe transformer's
FLOAT()callback converts everyFLOATtoken to a Python float —correct for a delay, destructive for condition text, where
0and0.0are differenttokens to a Verilog consumer.
Fix
Grammar only; no Python changes.
-equation: (operator | STRING | FLOAT | SCALARCONSTANT)+ +equation: (operator | STRING | EQNUMBER | SCALARCONSTANT)+ -SCALARCONSTANT: /[01]?'[bB][01]/ +SCALARCONSTANT.3: /[01]?'[bB][01]/ +// Numbers inside a condition keep their original text (see equation) +EQNUMBER.2: /[-+]?[0-9]+(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?/Conditions get their own number terminal, so
equationnever touchesFLOATand noconversion callback fires. This works because LALR mode uses a contextual lexer:
EQNUMBERis only reachable inside
equation, so delay positions still lex asFLOATexactly asbefore.
Three details that matter if this is edited later:
SCALARCONSTANTmust outrankEQNUMBER(.3vs.2). Reversed, the number terminaltakes the leading
1and1'b0splits again.EQNUMBERmust cover decimals, orA==1.5splits into1and.5.EQNUMBERneeds[-+]?, orA == -1fails to parse —FLOAT's-?had been carryingthat case. This also makes
+2work, whichFLOATnever allowed.Golden files
Two goldens are regenerated. The diff is the corruption being undone:
8 lines in
golden/fixpoint.sdf, 8 ingolden/spec-example2.sdf. No other golden changes.Verification
A=='b1was already correct and is unchanged.not what parses.