Add API Annotations
ClosedPublic

Authored by alanz on Nov 4 2014, 3:15 PM.

Details

Summary

The final design and discussion is captured at
https://ghc.haskell.org/trac/ghc/wiki/GhcAstAnnotations

This is a proof of concept implementation of a completely
separate annotation structure, populated in the parser,and tied to the
AST by means of a virtual "node-key" comprising the surrounding
SrcSpan and a value derived from the specific constructor used for the
node.

The key parts of the design are the following.

The Annotations

In hsSyn/ApiAnnotation.hs

type ApiAnns = (Map.Map ApiAnnKey SrcSpan, Map.Map SrcSpan [Located Token])

type ApiAnnKey = (SrcSpan,AnnKeywordId)

-- ---------------------------------------------------------------------

-- | Retrieve an annotation based on the @SrcSpan@ of the annotated AST
-- element, and the known type of the annotation.
getAnnotation :: ApiAnns -> SrcSpan -> AnnKeywordId -> Maybe SrcSpan
getAnnotation (anns,_) span ann = Map.lookup (span,ann) anns

-- |Retrieve the comments allocated to the current @SrcSpan@
getAnnotationComments :: ApiAnns -> SrcSpan -> [Located Token]
getAnnotationComments (_,anns) span =
  case Map.lookup span anns of
    Just cs -> cs
    Nothing -> []

-- | Note: in general the names of these are taken from the
-- corresponding token, unless otherwise noted
data AnnKeywordId
         = AnnAs
         | AnnBang
         | AnnClass
         | AnnClose -- ^ } or ] or ) or #) etc
         | AnnComma
         | AnnDarrow
         | AnnData
         | AnnDcolon
         ....

Capturing in the lexer/parser

The annotations are captured in the lexer / parser by extending PState to include a field

In parser/Lexer.x

data PState = PState {
        ....
        annotations :: [(ApiAnnKey,SrcSpan)]
        -- Annotations giving the locations of 'noise' tokens in the
        -- source, so that users of the GHC API can do source to
        -- source conversions.
     }

The lexer exposes a helper function to add an annotation

addAnnotation :: SrcSpan -> Ann -> SrcSpan -> P ()
addAnnotation l a v = P $ \s -> POk s {
  annotations = ((AK l a), v) : annotations s
  } ()

The parser also has some helper functions of the form

type MaybeAnn = Maybe (SrcSpan -> P ())

gl = getLoc
gj x = Just (gl x)

ams :: Located a -> [MaybeAnn] -> P (Located a)
ams a@(L l _) bs = (mapM_ (\a -> a l) $ catMaybes bs) >> return a

This allows annotations to be captured in the parser by means of

ctypedoc :: { LHsType RdrName }
        : 'forall' tv_bndrs '.' ctypedoc {% hintExplicitForall (getLoc $1) >>
                                            ams (LL $ mkExplicitHsForAllTy $2 (noLoc []) $4)
                                                [mj AnnForall $1,mj AnnDot $3] }
        | context '=>' ctypedoc         {% ams (LL $ mkQualifiedHsForAllTy   $1 $3)
                                               [mj AnnDarrow $2] }
        | ipvar '::' type               {% ams (LL (HsIParamTy (unLoc $1) $3))
                                               [mj AnnDcolon $2] }
        | typedoc                       { $1 }

Parse result

lang-haskell
data HsParsedModule = HsParsedModule {
    hpm_module    :: Located (HsModule RdrName),
    hpm_src_files :: [FilePath],
       -- ^ extra source files (e.g. from #includes).  The lexer collects
       -- these from '# <file> <line>' pragmas, which the C preprocessor
       -- leaves behind.  These files and their timestamps are stored in
       -- the .hi file, so that we can force recompilation if any of
       -- them change (#3589)
    hpm_annotations :: ApiAnns
  }

-- | The result of successful parsing.
data ParsedModule =
  ParsedModule { pm_mod_summary   :: ModSummary
               , pm_parsed_source :: ParsedSource
               , pm_extra_src_files :: [FilePath]
               , pm_annotations :: ApiAnns }

This diff depends on D426

Test Plan

sh ./validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alanz updated this revision to Diff 1251.Nov 4 2014, 3:15 PM
alanz retitled this revision from to Add annotations.
alanz updated this object.
alanz edited the test plan for this revision. (Show Details)
alanz added reviewers: austin, simonpj.
alanz updated the Trac tickets for this revision.
alanz retitled this revision from Add annotations to Add API Annotations.Nov 4 2014, 3:16 PM
alanz edited edge metadata.
simonpj accepted this revision.Nov 6 2014, 9:17 AM
simonpj edited edge metadata.

OK modulo doucmentation (see my comments) and perhaps using a new module.

I have not really studied the way you have implemented this in the parser, but it it's bound to be invasive. Maybe someone else can look at that

Simon

compiler/main/GHC.hs
725

Another golden opportuinty to link to that Note

compiler/parser/Lexer.x
1692

At the risk of sounding repetitive, a data type declaration is GOLDEN opportunity to say what is going on. Write a Note and mention it in several places.

1696

Ditto here

1697

And here

2600

I wonder if you might want to spliit the ApiAnns data type and its (pure) helper functions off into a separate module? it seems odd to import Lexer in other parts of the compiler. And it provides a great place for the Note to descirbe the data type and key ideas, which you can cite from elsewhere

2636

How can I tell which of these constructors should be available for any particular HsSyn construct?

You may say that it's obvious and usually it will be, but if I'm in doubt, how do I find out?

Perhaps you can give a Note that explains how, using a concrete example? Something like "look at the parser action for that construct; it will look like this... etc".

Simon

compiler/parser/Parser.y.pp
2808 ↗(On Diff #1251)

Can you give a Note to explain the general pattern, and how it works?

alanz updated this revision to Diff 1362.Nov 9 2014, 1:15 PM
alanz edited edge metadata.
  • Capture the end of file position in an annotation
  • Update for changes in the AST
alanz added a comment.Nov 9 2014, 1:26 PM

I am finally starting the documentation pass.

First question: should the type definitions in Lexer.x move to a module parser/ApiAnnotations.hs?

alanz updated this revision to Diff 1375.Nov 10 2014, 9:30 AM
  • Change MaybeAnn to AddAnn
  • Starting on documentation
  • Create ApiAnnotation module, instead of using Lexer.x
  • Improve documentation
alanz updated this revision to Diff 1376.Nov 10 2014, 9:44 AM
  • Add missing note
  • Note reason for trailing whitespace in test file
alanz updated this revision to Diff 1388.Nov 10 2014, 4:02 PM
  • Add location to WarningTxt

In an earlier comment I wrote "How can I tell which of these constructors should be available for any particular HsSyn construct?

You may say that it's obvious and usually it will be, but if I'm in doubt, how do I find out?

Perhaps you can give a Note that explains how, using a concrete example? Something like "look at the parser action for that construct; it will look like this... etc".

Did you respond to this? I can see it's tricky to do so.

Simon

alanz added a comment.Nov 11 2014, 5:43 AM

This is my next task, I realised I had not done this last night, and
updated Trac #9628 accordingly.

But I see it did not get submitted until I refreshed the page now.

Should these be haddock comments, so they show up in the web documentation
too?

Should these be haddock comments, so they show up in the web documentation
too?

I honestly don't know the best way to document this. Put yourself in the shoes of a user (which you are) and ask what you'd like to see. Maybe a couple of examples, and a pointer to a table giving comprehensive info? It's up to you

alanz added a comment.Nov 11 2014, 5:55 AM

My standard reference as a user is
https://www.haskell.org/ghc/docs/7.8.3/html/libraries/ghc-7.8.3/GHC.html
(or local version).

I will see if I can add haddock comments to the end of each constructor,
where applicable

alanz added a comment.Nov 11 2014, 8:33 AM

{F33547}Proposed haddock documentation for the annotations

compiler/parser/Lexer.x
2600

Done

alanz updated this revision to Diff 1406.Nov 11 2014, 4:16 PM
  • Starting to add haddock comments for annotations.
  • More documentation, and missing / overlapping annotations
alanz updated this revision to Diff 1513.Nov 18 2014, 2:08 PM
  • Add locations to AST elements to prepare for API annotations
alanz updated this revision to Diff 1515.Nov 18 2014, 2:53 PM
  • Add locations to AST elements to prepare for API annotations
alanz updated this revision to Diff 1516.Nov 18 2014, 3:04 PM

Squashed commit

alanz updated this revision to Diff 1559.Nov 19 2014, 3:45 PM

Tweaking SrcSpan in impspec

alanz updated this revision to Diff 1601.Nov 20 2014, 2:19 PM

Rebase for pattern synonym type signatures

alanz updated this revision to Diff 1604.Nov 20 2014, 3:08 PM

Fix haddock rebase

Mikolaj added a subscriber: Mikolaj.
Mikolaj added inline comments.
compiler/parser/Parser.y
124

A misclick?

Mikolaj accepted this revision.Nov 21 2014, 6:28 AM
Mikolaj edited edge metadata.

A very shallow review, mainly looking for typos and stray hunks from rebase and I've only found a trivial, benign one.

alanz updated this revision to Diff 1629.Nov 21 2014, 10:18 AM
alanz edited edge metadata.

Rebase

alanz updated this revision to Diff 1633.Nov 21 2014, 11:15 AM

Fix silly bracket error in parser

This revision was automatically updated to reflect the committed changes.