diff --git a/src/Parlot/Fluent/OneOf.cs b/src/Parlot/Fluent/OneOf.cs index b1e2f2c..2ea3882 100644 --- a/src/Parlot/Fluent/OneOf.cs +++ b/src/Parlot/Fluent/OneOf.cs @@ -1,10 +1,7 @@ -using FastExpressionCompiler; -using Parlot.Compilation; using Parlot.Rewriting; using System; using System.Collections.Generic; using System.Linq; -using System.Linq.Expressions; namespace Parlot.Fluent; @@ -13,7 +10,7 @@ namespace Parlot.Fluent; /// We then return the actual result of each parser. /// /// -public sealed class OneOf : Parser, ICompilable, ISeekable +public sealed class OneOf : Parser, ISeekable /*, ICompilable*/ { // Used as a lookup for OneOf to find other OneOf parsers that could // be invoked when there is no match. @@ -22,8 +19,9 @@ public sealed class OneOf : Parser, ICompilable, ISeekable internal readonly CharMap>>? _map; internal readonly List>? _otherParsers; - private readonly CharMap>> _lambdaMap = new(); - private Func>? _lambdaOtherParsers; + // For compilation, ignored for now + //private readonly CharMap>> _lambdaMap = new(); + //private Func>? _lambdaOtherParsers; public OneOf(Parser[] parsers) { @@ -63,14 +61,21 @@ public OneOf(Parser[] parsers) foreach (var c in seekable.ExpectedChars) { + IReadOnlyList> subParsers = parser is OneOf oneof ? oneof._map?[c] ?? [parser] : [parser!]; + if (c != OtherSeekableChar) { - lookupTable[c].Add(decoratedParser); + lookupTable[c].AddRange(subParsers); } else { _otherParsers ??= []; - _otherParsers.Add(decoratedParser); + _otherParsers.AddRange(subParsers!); + + foreach (var entry in lookupTable) + { + entry.Value.AddRange(subParsers); + } } } } @@ -197,6 +202,9 @@ public override bool Parse(ParseContext context, ref ParseResult result) return false; } +/* We don't use the ICompilable interface anymore since the generated code is still slower than the original one. + * Furthermore the current implementation is creating too many lambdas (there might be a bug in the code). + public CompilationResult Compile(CompilationContext context) { var result = context.CreateCompilationResult(); @@ -543,4 +551,5 @@ void UseSwitch() return result; } +*/ } diff --git a/test/Parlot.Tests/RewriteTests.cs b/test/Parlot.Tests/RewriteTests.cs index e518097..957de3b 100644 --- a/test/Parlot.Tests/RewriteTests.cs +++ b/test/Parlot.Tests/RewriteTests.cs @@ -1,5 +1,7 @@ using Parlot.Fluent; using Parlot.Rewriting; +using System.Collections.Generic; +using System.Reflection; using Xunit; using static Parlot.Fluent.Parsers; using static Parlot.Tests.Models.RewriteTests; @@ -149,20 +151,21 @@ public void OneOfIsSeekableIfAllAreSeekable() } - [Fact] - public void OneOfCanForwardSeekable() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void OneOfCanForwardSeekable(bool compiled) { // OneOf can create a lookup table based on ISeekable. - // However it can only be an ISeekable itself if all its parsers are. - // If one is not, then the caller would not be able to invoke it. - // This test ensures that such a parser is correctly invoked. + // It can bee seekable even if one of its parsers is not seekable. + // In that case it creates a custom `\0` with the non-seekable parsers. var pa = new FakeParser { CanSeek = true, ExpectedChars = ['a'], Success = true, Result = "a" }; var pb = new FakeParser { CanSeek = false, ExpectedChars = ['b'], Success = true, Result = "b" }; var pc = new FakeParser { CanSeek = true, ExpectedChars = ['c'], Success = true, Result = "c" }; var pd = new FakeParser { CanSeek = false, ExpectedChars = ['d'], Success = true, Result = "d" }; - // These ones are seekable because one is at least. + // These ones are seekable because one is at least. var p1 = OneOf(pa, pb); var p2 = OneOf(pc, pd); @@ -173,15 +176,46 @@ public void OneOfCanForwardSeekable() var p3 = OneOf(p1, p2); + if (compiled) p3 = p3.Compile(); + Assert.Equal("a", p3.Parse("a")); Assert.Equal("b", p3.Parse("b")); - Assert.Equal("c", p3.Parse("c")); + Assert.Equal("b", p3.Parse("c")); // p1's non-seekable are invoked, and pb is always successful Assert.Equal("b", p3.Parse("d")); pb.Success = false; Assert.Equal("a", p3.Parse("a")); - Assert.Equal("d", p3.Parse("b")); + Assert.Equal("d", p3.Parse("b")); // p1's non-seekable are invoked, now fail, p2 is invoked, 'c' doesn't match so pd is invoked and succeeds + Assert.Equal("c", p3.Parse("c")); + Assert.Equal("d", p3.Parse("d")); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void OneOfShouldFoldOneOfs(bool compiled) + { + // Recursive one-ofs should build a lookup table that is a combination of all the lookups. + // There should be a single lookup to find the best match + + var pa = new FakeParser { CanSeek = true, ExpectedChars = ['a'], Success = true, Result = "a" }; + var pb = new FakeParser { CanSeek = true, ExpectedChars = ['b'], Success = true, Result = "b" }; + var pc = new FakeParser { CanSeek = true, ExpectedChars = ['c'], Success = true, Result = "c" }; + var pd = new FakeParser { CanSeek = true, ExpectedChars = ['d'], Success = true, Result = "d" }; + + var p1 = OneOf(pa, pb); + var p2 = OneOf(pc, pd); + + var p3 = OneOf(p1, p2); + + Assert.True(p3 is ISeekable seekable && seekable.CanSeek); + Assert.Equal(['a', 'b', 'c', 'd'], ((ISeekable)p3).ExpectedChars); + + if (compiled) p3 = p3.Compile(); + + Assert.Equal("a", p3.Parse("a")); + Assert.Equal("b", p3.Parse("b")); Assert.Equal("c", p3.Parse("c")); Assert.Equal("d", p3.Parse("d")); }