tree-sitter / tree-sitter-haskell

Haskell grammar for tree-sitter.
MIT License
152 stars 36 forks source link

add type splices #30

Closed tek closed 3 years ago

tek commented 3 years ago

This adds support for splices in types.

Reason why I PR this is that this single rule addition causes an increase in generation time from four to twelve seconds :scream: and the resulting parser is 50% larger.

If anyone knows a way to avoid this, please advise.

tek commented 3 years ago

@maxbrunsfeld @patrickt ping!

patrickt commented 3 years ago

Hmm, I’m not sure. @maxbrunsfeld might have some observations?

maxbrunsfeld commented 3 years ago

I don't have much knowledge of this, but I remember that last time we were developing this grammar, the grammar became more difficult to develop as we started adding Template Haskell features. To me, the benefits of merging this as-is don't seem to outweigh the drawbacks.

I'm guessing this adds a lot of complexity because expressions can now occur as part of a type. I wonder if it'd be worth adding a more restricted form of splices (such as a specific splice syntax that must start with $( and end with )). Alternatively (or additionally), could splices be allowed in a more restricted set of positions, for example, only as the "outermost" type in a declaration? I'm not sure if either of these would work for a large fraction of the use cases in the wild.

tek commented 3 years ago

@maxbrunsfeld Indeed, the problem disappears if only parenthesized splices are allowed.

So the problem with TH as you recollect is mostly due to the fact that splices without dollar may occur at top level. The problem here is due to my using aexp as the expression that follows the dollar, which is, now that I look at it, terribly wrong.

Unfortunately, the GHC doc does not specify what precisely is allowed, it even says "arbitrary expression" iirc, but I think I can probably figure out a sensible set quite easily (just varids and bracketed stuff).

tek commented 3 years ago

alright, closing this since the problem is fixed and I can just make a simple commit.