Monoid for SrcSpan and SrcSpanInfo?

haskell-suite / haskell-src-exts

Manipulating Haskell source: abstract syntax, lexer, parser, and pretty-printer

Other

193 stars 94 forks source link

Monoid for SrcSpan and SrcSpanInfo? #48

Open mgsloan opened 11 years ago

mgsloan commented 11 years ago

There're a couple things in the way of having a proper Monoid for SrcSpan and SrcSpanInfo:

A good representation for empty. The existence of noLoc = SrcLoc "" (-1) (-1) sets a precedent for this representing an empty location / interval.
If mergeSrcSpan had a special case for empty, then it would be a proper definition for mappend. combSpanInfo / (<++>) throw away srcInfoPoints, which isn't so nice. You could still have a proper mappend, though, if they preserved srcInfoPoints when combined with mempty.

Maybe it would be better to have an explicit representation for empty SrcLoc and SrcSpan? Main issue is that it would be quite a breaking change.

niklasbroberg commented 10 years ago

I lean towards the breaking change. We're up for a rather big major release anyway next time, so it feels like a time to Do It Right (tm).

UnkindPartition commented 10 years ago

An explicit representation for an empty SrcSpan looks like a good thing to me.

I have serious doubts regarding SrcLoc, though. Is there such a thing as an empty location?

grep tells that noLoc isn't used anywhere HSE, so I'm not sure about its usefulness.

pjonsson commented 10 years ago

@mgsloan: What is the status for this ticket, is there code for this somewhere that should be merged? I agree with all of Roman's points.

mgsloan commented 10 years ago

I haven't done any work on this. Feel free to start work on it! I might get around to it sometime, though, if this becomes important for my usage of HSE. I also agree with Roman's points.

pjonsson commented 10 years ago

I will not fix this before the next release. It doesn't seem to be a blocker for your needs either.