Skip to content

Commit dd6abe2

Browse files
sebastienrossebastienros
committed
Fix left associative position reset
1 parent 8806713 commit dd6abe2

5 files changed

Lines changed: 137 additions & 16 deletions

File tree

‎src/Parlot/Fluent/LeftAssociative.cs‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using Parlot.Compilation;
2-
using Parlot.Rewriting;
32
using Parlot.SourceGeneration;
43
using System;
54
using System.Collections.Generic;
@@ -42,10 +41,12 @@ public override bool Parse(ParseContext context, ref ParseResult<T> result)
4241
}
4342

4443
var value = result.Value;
44+
var end = result.End;
4545

4646
// Parse zero or more (operator operand) pairs
4747
while (true)
4848
{
49+
var operatorPosition = context.Scanner.Cursor.Position;
4950
var operatorResult = new ParseResult<TInput>();
5051
Func<T, T, T>? matchedFactory = null;
5152

@@ -69,16 +70,17 @@ public override bool Parse(ParseContext context, ref ParseResult<T> result)
6970
var rightResult = new ParseResult<T>();
7071
if (!_parser.Parse(context, ref rightResult))
7172
{
72-
// Operator matched but no right operand - this is an error
73-
// For now we just stop parsing here
73+
// Operator matched but no right operand - rollback operator consumption.
74+
context.Scanner.Cursor.ResetPosition(operatorPosition);
7475
break;
7576
}
7677

7778
// Apply the operator
7879
value = matchedFactory(value, rightResult.Value);
80+
end = rightResult.End;
7981
}
8082

81-
result = new ParseResult<T>(result.Start, result.End, value);
83+
result = new ParseResult<T>(result.Start, end, value);
8284

8385
context.ExitParser(this);
8486
return true;
@@ -92,6 +94,7 @@ public CompilationResult Compile(CompilationContext context)
9294
var nextNum = context.NextNumber;
9395
var currentValue = result.DeclareVariable<T>($"leftAssocValue{nextNum}");
9496
var matchedFactory = result.DeclareVariable<Func<T, T, T>>($"matchedFactory{nextNum}");
97+
var operatorPosition = result.DeclareVariable<TextPosition>($"leftAssocPos{nextNum}");
9598

9699
var breakLabel = Expression.Label($"leftAssocBreak{nextNum}");
97100

@@ -152,13 +155,20 @@ public CompilationResult Compile(CompilationContext context)
152155
operatorChecks.AddRange(checkBlock);
153156
}
154157

158+
var scanner = Expression.Field(context.ParseContext, nameof(ParseContext.Scanner));
159+
var cursor = Expression.Field(scanner, nameof(Scanner.Cursor));
160+
var cursorPosition = Expression.Property(cursor, nameof(Cursor.Position));
161+
var resetPosition = typeof(Cursor).GetMethod(nameof(Cursor.ResetPosition), [typeof(TextPosition).MakeByRefType()])!;
162+
155163
// Build the loop body with its own variable scope
156164
var loopBody = Expression.Block(
157165
// Include operator variables and right parser variables in the loop scope
158166
allOperatorVariables.Concat(rightParserResult.Variables),
159167
new Expression[] {
160168
// Reset matchedFactory
161-
Expression.Assign(matchedFactory, Expression.Constant(null, typeof(Func<T, T, T>)))
169+
Expression.Assign(matchedFactory, Expression.Constant(null, typeof(Func<T, T, T>))),
170+
// Capture cursor position before attempting to match an operator
171+
Expression.Assign(operatorPosition, cursorPosition)
162172
}
163173
.Concat(operatorChecks)
164174
.Concat([
@@ -173,7 +183,10 @@ public CompilationResult Compile(CompilationContext context)
173183
// If right operand failed, break
174184
Expression.IfThen(
175185
Expression.Not(rightParserResult.Success),
176-
Expression.Break(breakLabel)
186+
Expression.Block(
187+
Expression.Call(cursor, resetPosition, operatorPosition),
188+
Expression.Break(breakLabel)
189+
)
177190
),
178191

179192
// Apply operator: currentValue = matchedFactory(currentValue, rightValue)
@@ -222,6 +235,7 @@ public SourceResult GenerateSource(SourceGenerationContext context)
222235

223236
var result = context.CreateResult(typeof(T));
224237
var ctx = context.ParseContextName;
238+
var cursorName = context.CursorName;
225239
var valueTypeName = SourceGenerationContext.GetTypeName(typeof(T));
226240
var inputTypeName = SourceGenerationContext.GetTypeName(typeof(TInput));
227241

@@ -265,6 +279,8 @@ static Type GetParserValueType(object parser)
265279
result.Body.Add(" while (true)");
266280
result.Body.Add(" {");
267281
result.Body.Add($" {operatorMatchedName} = false;");
282+
var operatorPositionName = $"opPos{context.NextNumber()}";
283+
result.Body.Add($" var {operatorPositionName} = {cursorName}.Position;");
268284

269285
// Generate operator matching for each operator
270286
for (int i = 0; i < _operators.Length; i++)
@@ -301,11 +317,10 @@ static Type GetParserValueType(object parser)
301317

302318
var innerIndent = i == 0 ? indent : $"{indent} ";
303319
result.Body.Add($"{innerIndent}{{");
304-
result.Body.Add($"{innerIndent} {operatorMatchedName} = true;");
305-
306320
// Parse right operand using helper
307321
result.Body.Add($"{innerIndent} if ({baseHelperName}({ctx}, out var {opResultName}RightValue))");
308322
result.Body.Add($"{innerIndent} {{");
323+
result.Body.Add($"{innerIndent} {operatorMatchedName} = true;");
309324
if (!context.DiscardResult)
310325
{
311326
result.Body.Add($"{innerIndent} {result.ValueVariable} = {factoryFieldName}({result.ValueVariable}, {opResultName}RightValue);");
@@ -315,6 +330,11 @@ static Type GetParserValueType(object parser)
315330
result.Body.Add($"{innerIndent} {factoryFieldName}({result.ValueVariable}, {opResultName}RightValue);");
316331
}
317332
result.Body.Add($"{innerIndent} }}");
333+
result.Body.Add($"{innerIndent} else");
334+
result.Body.Add($"{innerIndent} {{");
335+
result.Body.Add($"{innerIndent} {cursorName}.ResetPosition({operatorPositionName});");
336+
result.Body.Add($"{innerIndent} break;");
337+
result.Body.Add($"{innerIndent} }}");
318338
result.Body.Add($"{innerIndent}}}");
319339

320340
if (i > 0)

‎src/Parlot/Fluent/Unary.cs‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using FastExpressionCompiler;
22
using Parlot.Compilation;
3-
using Parlot.Rewriting;
43
using Parlot.SourceGeneration;
54
using System;
65
using System.Collections.Generic;
@@ -38,6 +37,7 @@ public override bool Parse(ParseContext context, ref ParseResult<T> result)
3837
// Try each unary operator
3938
foreach (var (op, factory) in _operators)
4039
{
40+
var operatorPosition = context.Scanner.Cursor.Position;
4141
var operatorResult = new ParseResult<TInput>();
4242
if (op.Parse(context, ref operatorResult))
4343
{
@@ -51,6 +51,7 @@ public override bool Parse(ParseContext context, ref ParseResult<T> result)
5151
else
5252
{
5353
// Operator matched but no operand - fail
54+
context.Scanner.Cursor.ResetPosition(operatorPosition);
5455
context.ExitParser(this);
5556
return false;
5657
}
@@ -88,6 +89,7 @@ public CompilationResult Compile(CompilationContext context)
8889
var nextNum = innerContext.NextNumber;
8990
var matchedFactory = innerResult.DeclareVariable<Func<T, T>>($"unaryFactory{nextNum}");
9091
var operatorMatched = innerResult.DeclareVariable<bool>($"unaryOpMatched{nextNum}");
92+
var operatorPosition = innerResult.DeclareVariable<TextPosition>($"unaryPos{nextNum}");
9193

9294
// Compile the base parser
9395
var baseParserResult = _parser.Build(innerContext);
@@ -144,6 +146,11 @@ public CompilationResult Compile(CompilationContext context)
144146
}
145147
}
146148

149+
var scanner = Expression.Field(innerContext.ParseContext, nameof(ParseContext.Scanner));
150+
var cursor = Expression.Field(scanner, nameof(Scanner.Cursor));
151+
var cursorPosition = Expression.Property(cursor, nameof(Cursor.Position));
152+
var resetPosition = typeof(Cursor).GetMethod(nameof(Cursor.ResetPosition), [typeof(TextPosition).MakeByRefType()])!;
153+
147154
// Build the recursive call - we'll use the closure to call ourselves
148155
var closureConst = Expression.Constant(_closure);
149156
var getFuncs = typeof(Closure).GetMember(nameof(Closure.Func))[0];
@@ -160,7 +167,9 @@ public CompilationResult Compile(CompilationContext context)
160167
new Expression[] {
161168
// Reset operator matched
162169
Expression.Assign(operatorMatched, Expression.Constant(false)),
163-
Expression.Assign(matchedFactory, Expression.Constant(null, typeof(Func<T, T>)))
170+
Expression.Assign(matchedFactory, Expression.Constant(null, typeof(Func<T, T>))),
171+
// Capture cursor position before attempting to match an operator
172+
Expression.Assign(operatorPosition, cursorPosition)
164173
}
165174
.Concat(operatorCheckExpressions)
166175
.Concat([
@@ -169,12 +178,13 @@ public CompilationResult Compile(CompilationContext context)
169178
// Operator matched - try recursive parse
170179
Expression.Block(
171180
Expression.Assign(recursiveResult, Expression.Invoke(castFunc, innerContext.ParseContext)),
172-
Expression.IfThen(
181+
Expression.IfThenElse(
173182
Expression.Field(recursiveResult, "Item1"),
174183
Expression.Block(
175184
Expression.Assign(innerResult.Success, Expression.Constant(true)),
176185
Expression.Assign(innerResult.Value, Expression.Invoke(matchedFactory, Expression.Field(recursiveResult, "Item2")))
177-
)
186+
),
187+
Expression.Call(cursor, resetPosition, operatorPosition)
178188
)
179189
),
180190
// No operator - try base parser
@@ -254,6 +264,7 @@ public SourceResult GenerateSource(SourceGenerationContext context)
254264

255265
var result = context.CreateResult(typeof(T));
256266
var ctx = context.ParseContextName;
267+
var cursorName = context.CursorName;
257268
var valueTypeName = SourceGenerationContext.GetTypeName(typeof(T));
258269

259270
var operatorMatchedName = $"unaryOpMatched{context.NextNumber()}";
@@ -298,16 +309,19 @@ static Type GetParserValueType(object parser)
298309
.MethodName;
299310

300311
var opResultName = $"opResult{context.NextNumber()}";
312+
var opPositionName = $"unaryPos{context.NextNumber()}";
301313

302314
var indent = "";
303315
if (i == 0)
304316
{
317+
result.Body.Add($"{indent}var {opPositionName} = {cursorName}.Position;");
305318
result.Body.Add($"{indent}if ({opHelperName}({ctx}, out _))");
306319
}
307320
else
308321
{
309322
result.Body.Add($"{indent}if (!{operatorMatchedName})");
310323
result.Body.Add($"{indent}{{");
324+
result.Body.Add($"{indent} var {opPositionName} = {cursorName}.Position;");
311325
result.Body.Add($"{indent} if ({opHelperName}({ctx}, out _))");
312326
}
313327

@@ -328,6 +342,10 @@ static Type GetParserValueType(object parser)
328342
result.Body.Add($"{innerIndent} {factoryFieldName}({opResultName}RecursiveValue);");
329343
}
330344
result.Body.Add($"{innerIndent} }}");
345+
result.Body.Add($"{innerIndent} else");
346+
result.Body.Add($"{innerIndent} {{");
347+
result.Body.Add($"{innerIndent} {cursorName}.ResetPosition({opPositionName});");
348+
result.Body.Add($"{innerIndent} }}");
331349
result.Body.Add($"{innerIndent}}}");
332350

333351
if (i > 0)

‎test/Parlot.SourceGenerator.Tests/GrammarsTests.cs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using Parlot.Tests.Calc;
2-
using Parlot;
32
using Parlot.Fluent;
43
using System;
54
using System.Threading;
@@ -129,6 +128,37 @@ public void NestedLeftAssociativeParser_InterceptsMethodCall()
129128
Assert.Equal(2.5, result); // (20 / 4) / 2 = 5 / 2 = 2.5
130129
}
131130

131+
[Fact]
132+
public void GeneratedLeftAssociative_RestoresCursorWhenRightOperandMissing()
133+
{
134+
var parser = Grammars.LeftAssociativeAdditionParser();
135+
136+
var context = new ParseContext(new Scanner("1+"));
137+
var result = new ParseResult<decimal>();
138+
139+
Assert.True(parser.Parse(context, ref result));
140+
Assert.Equal(1m, result.Value);
141+
Assert.Equal(0, result.Start);
142+
Assert.Equal(1, result.End);
143+
144+
// The trailing '+' must not be consumed if the RHS doesn't parse.
145+
Assert.Equal(1, context.Scanner.Cursor.Offset);
146+
}
147+
148+
[Fact]
149+
public void GeneratedUnary_RestoresCursorWhenOperandMissing()
150+
{
151+
var parser = Grammars.UnaryNegateDecimalParser();
152+
153+
var context = new ParseContext(new Scanner("-"));
154+
var result = new ParseResult<decimal>();
155+
156+
Assert.False(parser.Parse(context, ref result));
157+
158+
// The '-' must not be consumed on failure.
159+
Assert.Equal(0, context.Scanner.Cursor.Offset);
160+
}
161+
132162
[Fact]
133163
public void CalculatorParser_InterceptsMethodCall()
134164
{

‎test/Parlot.Tests/CompileTests.cs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using Parlot.Fluent;
22
using System;
33
using System.Collections.Generic;
4-
using System.Linq;
54
using System.Numerics;
65
using Xunit;
76
using static Parlot.Fluent.Parsers;
@@ -668,6 +667,34 @@ public void SelectShouldCompileHonorConcreteParseContext()
668667
Assert.Equal("no", no);
669668
}
670669

670+
[Fact]
671+
public void LeftAssociativeCompiledShouldRestoreCursorWhenRightOperandMissing()
672+
{
673+
var parser = Terms.Decimal().LeftAssociative((Terms.Char('+'), static (a, b) => a + b)).Compile();
674+
675+
var context = new ParseContext(new Scanner("1+"));
676+
var result = new ParseResult<decimal>();
677+
678+
Assert.True(parser.Parse(context, ref result));
679+
Assert.Equal(1m, result.Value);
680+
Assert.Equal(0, result.Start);
681+
Assert.Equal(1, result.End);
682+
683+
Assert.Equal(1, context.Scanner.Cursor.Offset);
684+
}
685+
686+
[Fact]
687+
public void UnaryCompiledShouldRestoreCursorWhenOperandMissing()
688+
{
689+
var parser = Terms.Decimal().Unary((Terms.Char('-'), static d => -d)).Compile();
690+
691+
var context = new ParseContext(new Scanner("-"));
692+
var result = new ParseResult<decimal>();
693+
694+
Assert.False(parser.Parse(context, ref result));
695+
Assert.Equal(0, context.Scanner.Cursor.Offset);
696+
}
697+
671698
[Fact]
672699
public void ShouldCompileTextBefore()
673700
{

‎test/Parlot.Tests/FluentTests.cs‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
using System;
44
using System.Collections.Generic;
55
using System.Globalization;
6-
using System.Linq;
76
using System.Numerics;
8-
using System.Runtime.InteropServices;
97
using Xunit;
108

119
using static Parlot.Fluent.Parsers;
@@ -1145,6 +1143,22 @@ public void ShouldParseLeftAssociativity(string expression, double result)
11451143
Assert.Equal(result, multiplicative.Parse(expression));
11461144
}
11471145

1146+
[Fact]
1147+
public void LeftAssociativeShouldRestoreCursorWhenRightOperandMissing()
1148+
{
1149+
var parser = Terms.Decimal().LeftAssociative((Terms.Char('+'), static (a, b) => a + b));
1150+
1151+
var context = new ParseContext(new Scanner("1+"));
1152+
var result = new ParseResult<decimal>();
1153+
1154+
Assert.True(parser.Parse(context, ref result));
1155+
Assert.Equal(1m, result.Value);
1156+
Assert.Equal(0, result.Start);
1157+
Assert.Equal(1, result.End);
1158+
1159+
Assert.Equal(1, context.Scanner.Cursor.Offset);
1160+
}
1161+
11481162
[Theory]
11491163
[InlineData("2", 2)]
11501164
[InlineData("-2", -2)]
@@ -1160,6 +1174,18 @@ public void ShouldParsePrefix(string expression, double result)
11601174
Assert.Equal(result, unary.Parse(expression));
11611175
}
11621176

1177+
[Fact]
1178+
public void UnaryShouldRestoreCursorWhenOperandMissing()
1179+
{
1180+
var parser = Terms.Decimal().Unary((Terms.Char('-'), static d => -d));
1181+
1182+
var context = new ParseContext(new Scanner("-"));
1183+
var result = new ParseResult<decimal>();
1184+
1185+
Assert.False(parser.Parse(context, ref result));
1186+
Assert.Equal(0, context.Scanner.Cursor.Offset);
1187+
}
1188+
11631189
[Fact]
11641190
public void ShouldZeroOrOne()
11651191
{

0 commit comments

Comments
 (0)