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 aboutAnchorReal
andAnchorDelta
?
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
AnnAnchor
s areAR
s. So just panic in theAD
case. -
@rae started a discussion: Why will some AST nodes use
ApiAnn
and other use custom structures, like theAnnsLet
example, above? It seems like thisApiAnn
, storing a list ofAddApiAnn
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
andAnchor
. 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
'
. TheApiAnn
synonym 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_KeepRawTokenStream
is no longer used. Remove it. Or do we need some mechanism to turn of keeping the EPAs?
Captured in #19706
- []