Enabling scoping mechanism #35

npenin · 2021-03-16T20:00:30Z

Hi seb,

One more pull request that allows scoped parsers (that required an adaptation to the parsercontext). That allows registering variable declarations in a block statement before actually waiting for the parser to complete

sebastienros · 2021-03-16T20:29:54Z

Not opposed to it. Right now the solution is to pass a custom instance inheriting from ParserContext and cast it in a Then() call. See https://github.com/sebastienros/fluid/blob/main/Fluid/FluidParser.cs#L193

npenin · 2021-03-17T08:49:11Z

Maybe I missed something, but I see 1 major problem with this: how do you handle nested scopes ? the approach I suggested is most likely not the best, but it at least helps solving this problem which sounds like a common use case for parsers.

src/Parlot/Fluent/ScopedParser.cs

splitted ParseContext and ParseContext.Untyped

lahma · 2021-03-20T12:21:24Z

Generic TParseContext will also probably open possibility to use struct type by restricting to new IParseContext interface and consumers like Fluid can use their own type.

npenin · 2021-03-22T07:19:07Z

The other "small" change I made is to make an IParser interface for better variance/covariance (I never remember which one is which) support which, does not work well with classes.

sebastienros · 2021-03-24T18:23:39Z

@npenin That's great! I had to remove the interfaces at some point because it was preventing me from doing some Fluent things in places. I had an IParser and all classes had to implement it, but it would make them have to return an object and that was a pain.

for better variance/covariance

Had a quick look and I couldn't find that. Ideally IParser<T> should be castable to IParser<object>. That would allow to remove some Then<> calls to have all parsers in a sequence to return the same exact type.

Can you split this PR in two, one with the Scoped parser and one with the interfaces? I want to deal with the changes independently.

npenin · 2021-03-24T18:59:33Z

Unfortunately, I don't think so. If I split it, it will have to still contain all my changes for the typed parsercontext, otherwise, that would be a completely different implementation.

Regarding the variance/covariance, there is a flaw in .NET that would prevent from casting a Task<IList> to Task<IEnumerable> because Task is a class and variance/covariance only works with interfaces (and delegates ?).

…ope2

sebastienros · 2021-04-30T18:24:19Z

Now that I have merged compilation I will be able to focus on your PRs.

npenin · 2021-04-30T18:26:41Z

it took me quite some time to get it compile again after your compilation changes. I hope I will not have to do it again ;)

…ope2

sebastienros · 2021-05-01T14:24:53Z

src/Parlot/Fluent/Then.cs

+    /// </summary>
+    /// <typeparam name="T">The input parser type.</typeparam>
+    /// <typeparam name="TParseContext">The parse context type.</typeparam>
+    public sealed class Then<T, TParseContext> : Parser<T, TParseContext>, ICompilable<TParseContext>


That's a new one right?
I assume to just be able to pass an action that can change the context only?

sebastienros · 2021-05-01T14:49:27Z

Ran the benchmarks, slower by 10%.

npenin · 2021-05-01T15:55:02Z

do you know where exactly or is it just a general slowness ?

sebastienros · 2021-05-01T15:57:04Z

Global regression due to interface dispatch, @lahma ?

sebastienros · 2021-05-01T15:58:17Z

Though if there is no possibility of using a "common" IParser interface or using co-variance, then why not just remove all interfaces like it was before and only keep the Parser<T, TParseContext> part?

lahma · 2021-05-02T04:49:00Z

Global regression due to interface dispatch, @lahma ?

That would be my guess. Interface dispatch is a lot more costly than regular virtual member call.

npenin · 2021-05-02T07:46:09Z

Indeed, I removed the interface and earned back the 10%.

sebastienros · 2021-05-07T18:27:57Z

https://twitter.com/badamczewski01/status/1390390252435611654

sebastienros · 2021-05-07T18:46:34Z

The new SkipWhiteSpace parser needs to be updated

sebastienros · 2021-05-09T21:08:10Z

src/Parlot/Fluent/Parser.Compile.cs

@@ -5,22 +5,31 @@

 namespace Parlot.Fluent
 {
-    public abstract partial class Parser<T>
+    public partial class Parsers


Like there is public static class ParserExtensions for TryParse it should be a ParserExtensions for Compile. Maybe both as partial classes.

sebastienros · 2021-05-09T21:17:01Z

src/Parlot/Fluent/ParseContext.UntypedScope.cs

+
+namespace Parlot.Fluent
+{
+    public partial class ParseContext


I don't understand why it should be part of the library. This PR allows to change the type of ParseContext, so any project could then create their own, and have one that takes a dictionary if they need, or custom properties.

indeed, I just thought having an example and the possiblity to use such a parser straight ahead would be the right thing to do. but I agree with you

src/Parlot/Fluent/ParseContext.Scoped.cs

sebastienros · 2021-05-09T21:23:25Z

src/Parlot/Fluent/ParseContext.cs

@@ -32,7 +32,7 @@ public ParseContext(Scanner scanner, bool useNewLines = false)
        /// <summary>
        /// The parser that is used to parse whitespaces and comments.
        /// </summary>
-        public Parser<TextSpan> WhiteSpaceParser { get; set;}
+        public Parser<TextSpan, ParseContext> WhiteSpaceParser { get; set; }


This looks like it could now be in a custom ParseContext that deals with char, since it doesn't make sense for byte based parsers. Then the Literals or Terms based parsers could use this new context type, to drive the other parsers generic type. There would then be a new set of binary parsers with another corresponding type.

Or maybe it's a Scanner concern too.

I think you mixed this comment with my other PR. BTW, it is done as such in my other PR (see StringParseContext).

npenin · 2021-10-05T09:17:00Z

@sebastienros up ?

added scoping mechanism to parsers

f63fa58

code formatting

4f76cea

sebastienros reviewed Mar 19, 2021

View reviewed changes

src/Parlot/Fluent/ScopedParser.cs Outdated Show resolved Hide resolved

implemented strongly typed parsecontext

ec9d71d

splitted ParseContext and ParseContext.Untyped

PENINNI2 added 2 commits April 30, 2021 19:45

Merge branch 'main' of https://github.com/sebastienros/parlot into sc…

795ed1e

…ope2

Merge main branch

0aaa2a6

PENINNI2 and others added 2 commits April 30, 2021 20:29

Merge branch 'main' of https://github.com/sebastienros/parlot into sc…

c691276

…ope2

fixed compilation

8cdf179

sebastienros reviewed May 1, 2021

View reviewed changes

Fixing benchmarks compilation

358143d

npenin and others added 2 commits May 2, 2021 09:46

removed interface to restore performances

f18fd9a

improved scoping usage

1d8c924

Merge branch 'main' into scope2

0eb62bd

fixed compilation after merge from main

42dafd4

npenin mentioned this pull request May 9, 2021

Typedbuffer and low level parsers #45

Open

sebastienros reviewed May 9, 2021

View reviewed changes

renamed ParseContext<> to ScopeParseContext

0ec9a68

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling scoping mechanism #35

Enabling scoping mechanism #35

npenin commented Mar 16, 2021

sebastienros commented Mar 16, 2021

npenin commented Mar 17, 2021

lahma commented Mar 20, 2021

npenin commented Mar 22, 2021

sebastienros commented Mar 24, 2021

npenin commented Mar 24, 2021 •

edited

Loading

sebastienros commented Apr 30, 2021

npenin commented Apr 30, 2021

sebastienros May 1, 2021

npenin May 1, 2021

sebastienros commented May 1, 2021

npenin commented May 1, 2021

sebastienros commented May 1, 2021

sebastienros commented May 1, 2021

lahma commented May 2, 2021

npenin commented May 2, 2021

sebastienros commented May 7, 2021

sebastienros commented May 7, 2021

sebastienros May 9, 2021

sebastienros May 9, 2021

npenin May 10, 2021

sebastienros May 9, 2021

npenin May 10, 2021

npenin commented Oct 5, 2021

Enabling scoping mechanism #35

Are you sure you want to change the base?

Enabling scoping mechanism #35

Conversation

npenin commented Mar 16, 2021

sebastienros commented Mar 16, 2021

npenin commented Mar 17, 2021

lahma commented Mar 20, 2021

npenin commented Mar 22, 2021

sebastienros commented Mar 24, 2021

npenin commented Mar 24, 2021 • edited Loading

sebastienros commented Apr 30, 2021

npenin commented Apr 30, 2021

sebastienros May 1, 2021

Choose a reason for hiding this comment

npenin May 1, 2021

Choose a reason for hiding this comment

sebastienros commented May 1, 2021

npenin commented May 1, 2021

sebastienros commented May 1, 2021

sebastienros commented May 1, 2021

lahma commented May 2, 2021

npenin commented May 2, 2021

sebastienros commented May 7, 2021

sebastienros commented May 7, 2021

sebastienros May 9, 2021

Choose a reason for hiding this comment

sebastienros May 9, 2021

Choose a reason for hiding this comment

npenin May 10, 2021

Choose a reason for hiding this comment

sebastienros May 9, 2021

Choose a reason for hiding this comment

npenin May 10, 2021

Choose a reason for hiding this comment

npenin commented Oct 5, 2021

npenin commented Mar 24, 2021 •

edited

Loading