search

dlang-community / Pegged

A Parsing Expression Grammar (PEG) module, using the D programming language.
534 stars 66 forks source link

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

Open cbleser opened 3 years ago

cbleser commented 3 years ago

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;
}
veelo commented 3 years ago

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.

veelo commented 3 years ago

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.