Skip to content

Follow-up from "Implement in-tree API Annotations"

The following discussions from !2418 (closed) should be addressed:

  • @rae started a discussion:

    What annotations_comments field?

    Updated in !5363 (merged)

  • @rae started a discussion:

    Another potential reference to an out-of-date resource.

  • @rae started a discussion:

    Hard to generalize {-# LANGUAGE. Is this always just the opening of a pragma? Then please say so.

This is updated in !5373 (closed)

  • @rae started a discussion:

    This needs more description. Why is this case special?

I have created #19697 (closed) to track the problems around the EpaEofComment. And will also update the comment in the code to refer to it.

  • @rae started a discussion:

    The prior token? But what tells us the location of the comment itself?

I have updated the comment to note that the EpaComment is GenLocated, and that location is used for the delta calculation.

  • @rae started a discussion:

    -- parsed source.  This can be replaced by the @'AD'@ version, to
  • @rae started a discussion:

    This description makes me want

    data DeltaPos
      = SameLine { deltaColumn :: !Int }
      | DifferentLine { deltaLine :: !Int, startColumn :: !Int }

    which seems less error-prone.

I captured this requirement in #19698 (closed).

  • @rae started a discussion:

    The pervasiveness of e.g. mj in the parser incentivizes a short name, even if it's inscrutable. But do these need to be similarly hard to read? How about AnchorReal and AnchorDelta?

Captured in #19699 (closed)

  • @rae started a discussion:

    This function is very suspicious. But it's actually OK: it's used only directly in the parser, where we can be sure (if I understand correctly) that all AnnAnchors are ARs. So just panic in the AD case.

  • @rae started a discussion:

    Why will some AST nodes use ApiAnn and other use custom structures, like the AnnsLet example, above? It seems like this ApiAnn, storing a list of AddApiAnn is sufficient (if over-expressive). So I guess why I see why you would want custom structures (making impossible states unrepresentable), but then why not just use custom structures everywhere?

This part is incomplete, and needs to be driven to use a common approach throughout. The specifics have now moved to #19623 (closed) and !5440 (closed).

  • @rae started a discussion:

    I don't have a good grasp on the difference between AnnAnchor and Anchor. Why sometimes use one and sometimes the other? They seem very similar.

I added a description of the use of Anchor in https://gitlab.haskell.org/ghc/ghc/-/wikis/api-annotations#mechanism, the key thing is that is always has a RealSrcSpan in it. The EpaAnchor has either the original absolute location, which is used to calculate a DeltaPos relative to the RealSrcSpan in the Anchor for the containing AST element, or the DeltaPos directly, if it has been constructed by a tool.

  • @rae started a discussion:

    I rather dislike the convention using this '. The ApiAnn synonym isn't much used. Maybe just kill it and then you can kill this '?

I agree, created #19705 (closed) to track it.

I will rename it to emptyComments, there is already a noComments. And there is already a Semigroup instance for EpAnnComments, we can make this mempty too.

  • @rae started a discussion:

    This should also be a longer name, I think.

com is not actually used, I have removed it.

Additional items

  • Opt_KeepRawTokenStream is no longer used. Remove it. Or do we need some mechanism to turn of keeping the EPAs?

Captured in #19706

  • []
Edited by Alan Zimmerman
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information