Skip to content

EPA: Document and/or restructure NameAnn

In reviewing some code, I decided to educate myself a bit on the new EPA structure, and I stumbled.

Very specifically: the EpaAnnNotUsed constructor says that it's used in generated code, such as TH or deriving. Yet, this is not true in practice: the sL1n function is used much in the parser of user-written code, and it calls noAnnSrcSpan, which uses EpAnnNotUsed. So, at a minimum, the comment on EpaAnnNotUsed should be updated.

But I think there is some restructuring that we could do here.

  • I like the idea of EpaAnnNotUsed, as currently documented (but not as used): it says when there is no useful EPA to give. But shouldn't it always be paired with UnhelpfulSpan? If I'm right -- that EpaAnnNotUsed and UnhelpfulSpan go together -- that suggests that the EpAnn/SrcSpanAnn' structure is mis-aligned. That is, maybe SrcSpanAnn' (what is that prime doing there?!) should have two constructors, one for real spans and one for unhelpful ones. This would be instead of giving EpAnn two constructors.

  • I was quite surprised reading NameAnn: I kept looking for a constructor that would apply for normal names. I had to poke around to find out that the normal case is covered by EpaAnnNotUsed... which (to me) doesn't communicate "normal name".

  • There's also something that feels off about the five difference SrcSpanAnn? types. In particular, there is NameAnn, useful for names, and AnnListItem, useful for things that can be stored in lists. But what if I have a list of names? This happens in fixity declarations, for example. Looking at data FixitySig -- which just stores a [LIdP pass] (which stores NameAnn) -- I don't see how it's possible to store the commas correctly. That is, how do we tell the difference between infix 5 *, + and infix 5 * ,+? I really don't know! And it seems impossible to store this with the current NameAnn/AnnListItem split. So something seems wrong here.

  • Perhaps related: each constructor of NameAnn has a nann_trailing field. Lots of other annotations have a similar field, too. This seems like something that should abstracted into its own definition.

Aha! I just unlocked new understanding. The last two points are intimately related. AnnListItem is, essentially, a least-common-denominator for these annotations, containing just [TrailingAnn] (still not 100% sure what that is). This relates to some wisdom I read somewhere, which said "If you need a Located, use LocatedA now." -- that's because AnnListItem is in some sense the least informative annotation. But because all sorts of other things can also appear in lists, all the other annotation forms also include the [TrailingAnn] -- in particular, NameAnn. So names can appear in a list: they just use the nann_trailing to act like the lann_trailing field of an AnnListItem.

This all strongly suggests that the [TrailingAnn] information is misplaced. Either it should be part of the enclosing list AST node somehow, or, failing that, it should be part of EpAnn, which frequently (always?) wraps the individual annotations (like NameAnn and AnnListItem). With this change, AnnListItem stores no information and can become a type isomorphic to ().

I'm sure there's more to work out here, but at the very very least, let's redocument EpaAnnNotUsed (and maybe rename it?) to say what it's really for. Thanks!

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information