Follow-up from "Implement in-tree API Annotations"
The following discussions from !2418 (closed) should be addressed:
-
@rae started a discussion: What
annotations_commentsfield?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.
mjin the parser incentivizes a short name, even if it's inscrutable. But do these need to be similarly hard to read? How aboutAnchorRealandAnchorDelta?
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 areARs. So just panic in theADcase. -
@rae started a discussion: Why will some AST nodes use
ApiAnnand other use custom structures, like theAnnsLetexample, above? It seems like thisApiAnn, storing a list ofAddApiAnnis 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
AnnAnchorandAnchor. 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
'. TheApiAnnsynonym isn't much used. Maybe just kill it and then you can kill this'?
I agree, created #19705 (closed) to track it.
-
@rae started a discussion: Please use a longer name:
noComments
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_KeepRawTokenStreamis no longer used. Remove it. Or do we need some mechanism to turn of keeping the EPAs?
Captured in #19706
- []