Feedback request for #9628 AST Annotations

[Alan & Kim Zimmerman]

Hi All

I am looking for some input on #9628 for source annotations.

The intent behind this is to make it easier for tool developers to add
extra information to the AST. This is to assist in structured editing
or other source to source conversions, such as refactoring.

One of the use cases would be to maintain the ParsedSource as tightly
coupled to the editor, where changes would be made directly to the AST
and only indirectly to the text source. This is the approach enabled
by structured haskell mode, or demonstrated in lamdu.

Ideally it should be possible to perform a change to the ParsedSource,
and then submit it to the renamer and typechecker for error and
warning feedback. As such the renamer / typechecker should be able to
accept the annotated AST.

Eventually this process should be incremental, since a given edit
takes place in a structured way in a single position at a time, so it
should be possible to identify which part of previously done work is
invalidated by it and needs to be redone.

This would happen in the context of a ghci session operating as a
server.

In order to lay the initial groundwork for this, I have updated the
AST in hsSyn to replace all of the `Located` elements with `GenLocated l`,
where `l` is constrained to be a member of class `ApiAnnotation`.

    class (Ord l, Outputable l, OutputableBndr l) => ApiAnnotation l where
        annGetSpan :: l -> SrcSpan
        annSetSpan :: l -> SrcSpan -> l
        annNoSpan  :: l

This guarantees that the annotation does in fact keep a `SrcSpan` in it.

I have also worked this through the renamer, typechecker, and
desugarer, and the monad for core.

I have kept the `Located` / `GenLocated SrcSpan` boundary for the parser,
and within the `RdrName`,`Name`,`Id` types.

In this context the hooks become especially important, but they need
to get the annotation type parameter.

It seemed extreme to force the `DynamicFlags` to have to have the type
parameter, so I am proposing to move the hooks to `HscEnv`, which needs
a type parameter, even as a phantom type, to make sure that the right
data structures are instantiated in the monads for the various passes.


This is a big change, and I am very much working in the dark, so I
would love some feedback or discussion.

1. Is this change too big, should I scale it back to just update the
   HsSyn structures and then lock it down to Located SrcSpan for all
   the rest?

2. Will the ApiAnnotation constraint and resolution cause performance
   degradation?

2. Is it ok to move the hooks into HscEnv?

3. In order to have a single session for a project, it needs a single
   annotation parameter. To support this the parser needs to produce
   annotated output. It should be a simple case of updating the
   L0/L1/LL macros and production types.  Does it make sense to do this?

This is intended to be an enabling change, without changing the
structure of any code or the AST. A later change would introduce
additional annotations in HsSyn to track the locations of the various
keywords and other punctuation so that exactly regenerating source is
much easier, without requiring extensive examiniation of the tokens.

References

1. https://ghc.haskell.org/trac/ghc/ticket/9628
2. https://ghc.haskell.org/trac/ghc/wiki/GhcAstAnnotations
3. https://phabricator.haskell.org/D246
4. https://github.com/alanz/ghc/commits/wip/ast-ann-location

Regards
  Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140929/5c39ec53/attachment.html>