Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inside peg.d the ParseTree change to a mixin template #302

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

bitcuits
Copy link

The suggestion is to change the ParseTree to a mixin template.
Like the following

mixin template ParseTreeT {
 .... ParseTree code
}

struct ParseTree {
       mixin ParseTreeT();
}

This makes it easier to add additional functions to the ParseTree struct.
Like.

struct MyParseTree {
     mixin ParseTreeT();
     bool smart_flag;
}

This enables 'overloading' of the ParseTree
@veelo
Copy link
Collaborator

veelo commented May 29, 2021

I think this is a good suggestion. However if I'm not mistaken, Pegged is currently not consistently templatized, meaning that in many places ParseTree is used directly. So I don't think the extensibility as advertised in the wiki actually works (but I may be mistaken). Is this change sufficient to extend ParseTree for your purposes? Otherwise I think it would be good to extend this PR with templatization until it is.

Thanks,
Bastiaan.

pegged/peg.d Outdated
@@ -49,7 +49,7 @@ package string stringified(string inp) @safe
"\t" : `"\t"`,

`"` : `"\""`,
"`" : q"("`")",
"`" : q{("`")},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You changed this from a delimited string to a token string, which broke the test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A long while ago I posted ANTLR runtime to D. But I think peg fits better to D.
But what I like with ANTLR is that you can do channels and I was thinking of some thinking similar in pegged.
But for now, I think we should close my suggestion for this change now.
I will try to make a better proposal so we can have some context ParseTree.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, you can close the PR yourself if you wish. I don't know what channels are in parsers, but I look forward to see your work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hej Veelo.
I have made an updated, so it should be possible to make a costume ParseTree.
It parses the unittest, but it is not tested yet. I will use it in my other project now so I will and in the process make a test for it.

Note.
Channel is used when you have syntax groups.
Like is C.
You have comments, macros, code, and maybe more.
So with channels, you can write a grammar that adds a channel to a part of the grammar.
When you traverse the tree you can select which part to traverse. Ex if you don' what to look at the comment you can filter to about.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what Pegged does is that it builds a parse tree as it processes the input. After that the user traverses the parse tree in order to interpret it and extract data from it. So you could say that there are two stages in a Pegged parsing process. At what stage would you select a particular channel? I mean, does selecting a channel affect the parse tree? If it is in the first stage, it seems to me to be somewhat like selectively dropping nodes. That should be possible today using semantic actions. If it is in the second stage, it would be up to the user to implement this, no?
There is also the feature of composable grammars, so you can have a grammar for comments and a grammar for code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hej Veelo.
I have added the option to customize the ParseTree.
At first glance, it seems that the code has changed a lot but in principle, I have just added some mixin templates and some introspection functions.
I have tried not to destroy your code ;-).
I have also changed so most of the code is now @safe.

In the makefile I have added a
make test-all -k
Which runs a dub test on all the examples.
The same tests parse for this branch as for the master.

And I have added a makefile inside the pegged/dev to regenerate the parser.d

I will now continue to use this version in more project and I will post you the result and some ex

Working on changing the peg grammers to use costume ParseTree datatypes
…t be declated a static member inside the ParseTree object
parser.r has been self gererated from pegged/dev/regenerate.d
A Makefile has been added to pegged/dev/, to build the regenerate
@veelo
Copy link
Collaborator

veelo commented Jun 10, 2021

Hi Carsten,

Just a quick note while your work is in progress, which by the way is much appreciated. I notice that the diff is getting quite big. I know it is tempting to improve code as soon as you see an opportunity, also when it is not directly related to the subject of the PR. However, it would greatly help me if you could make those changes in a separate branch from which you can create a separate PR. Examples are indentation changes, renamings, making things @safe and other refactorings. You can merge those changes back into your development branch so you directly get their benefits if you want, and if those changes are easy enough to review I can also merge them quickly. You can then rebase your development branch which reduces the diff of this PR. An added advantage is that Pegged improves with small steps irrespective whether your main WIP finalizes or not.

Another point is that there are corners in Pegged that never left the experimental stage, like dynamic grammars. If you experience that these corners are difficult to integrate in your work, it may be worth considering removing those. See also the discussion in #176. If so, we should have a discussion about that, including doing that in a separate PR (of course) and maybe in a completely different branch of the repository, and prepare a major version bump of Pegged. (OT: Another idea I would like to see checked out is whetter Pegged could support switching on ordinary enums instead of strings, and whether that would improve performance. This would also need a major version bump.)

My point is you are investing a lot of time in this, and I want it to be worthwhile and not go to waste. The bigger the diff, the harder that gets...

Anyway, keep it up and have fun!

Bastiaan.

make test-all

Examples.
src/pegged/examples/cparser.d
source/app.d
src/pegged/examples/constraints.d
Does not pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants