Opened 10 years ago

Closed 6 years ago

Last modified 6 years ago

#3725 closed bug (fixed)

Annotations not written to interface files

Reported by: rl Owned by:
Priority: normal Milestone: 7.8.1
Component: Compiler Version: 7.7
Keywords: Cc: gergely@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking: #7867
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Small example:

module C where

data T a = T a

Compile it:

newbie:tests rl$ ~/projects/ndp/ghc/inplace/bin/ghc-stage2 -O2 -c C.hs
newbie:tests rl$ ls -l C.hi
-rw-r--r--  1 rl  rl  485  4 Dec 12:53 C.hi

Add an annotation: {-# ANN type T () #-}. Compile:

newbie:tests rl$ ~/projects/ndp/ghc/inplace/bin/ghc-stage2 -O2 -c C.hs
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading package ffi-1.0 ... linking ... done.
newbie:tests rl$ ls -l C.hi
-rw-r--r--  1 rl  rl  485  4 Dec 12:53 C.hi

Note that the interface file hasn't been updated. Remove C.hi and recompile:

newbie:tests rl$ rm C.hi
newbie:tests rl$ ~/projects/ndp/ghc/inplace/bin/ghc-stage2 -O2 -c C.hs
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading package ffi-1.0 ... linking ... done.
newbie:tests rl$ ls -l C.hi
-rw-r--r--  1 rl  rl  507  4 Dec 12:54 C.hi

Only now has the annotation been written to C.hi.

Attachments (1)

ghc-3725.patch (13.4 KB) - added by errge 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by simonpj

Roman,

Quite right. The relevant code is in MkIface.lhs. Look at the declaration of IfaceDeclExtras:

data IfaceDeclExtras 
  = IfaceIdExtras    Fixity [IfaceRule]
  | IfaceDataExtras  Fixity [IfaceInstABI] [(Fixity,[IfaceRule])]
  | IfaceClassExtras Fixity [IfaceInstABI] [(Fixity,[IfaceRule])]
  | IfaceSynExtras   Fixity
  | IfaceOtherDeclExtras

This collects information that forms part of an Id's fingerprint, even though the information is not serialised along with the Id itself; instead it occurs elsewhere in the interface file. For example, fixity forms part of an Id's fingerprint, but appears in the mi_fixities field of the ModIface.

What we'd forgotten is that the annotations for an Id should form part of its "extras". So the IfaceIdExtras constructor should look like

  IfaceIdExtras Fixity [IfaceRule] [Serialized]

If you make that change, then I think the knock-on changes (notably filling in that field) will be obvious.

Could you have a go at doing this? Thanks

Simon

comment:2 Changed 10 years ago by igloo

Milestone: 6.12.2

comment:3 Changed 9 years ago by igloo

Milestone: 6.12.26.12.3

comment:4 Changed 9 years ago by igloo

Milestone: 6.12.36.14.1
Priority: normallow

comment:5 Changed 9 years ago by igloo

Milestone: 7.0.17.0.2

comment:6 Changed 9 years ago by igloo

Milestone: 7.0.27.2.1

comment:7 Changed 8 years ago by igloo

Milestone: 7.2.17.4.1

comment:8 Changed 8 years ago by igloo

Milestone: 7.4.17.6.1
Priority: lowlowest

comment:9 Changed 7 years ago by igloo

Milestone: 7.6.17.6.2

comment:10 Changed 6 years ago by errge

Blocking: 7867 added

(In #7867) Nevermind my previous comment, I found the relevant bug: http://hackage.haskell.org/trac/ghc/ticket/3725

OK, I will continue by investigating if it's possible to get this stuff from template haskell and if not, how hard would be to add it...

I'll get back to this ticket if I have or need further info.

Changed 6 years ago by errge

Attachment: ghc-3725.patch added

comment:11 Changed 6 years ago by errge

Cc: gergely@… added
difficulty: Easy (less than 1 hour)
Milestone: 7.6.27.8.1
Priority: lowestnormal
Status: newpatch
Version: 6.137.7

Well, it wasn't as knock-on as expected, but doable :)

Had to pay attention to a lot of other possibilities to define annotations:

  • module annotations,
  • type annotations (both class and data),
  • type class method annotations,
  • data constructor annotations.

Raising back priority to normal: this is kind of important for people who try to use annotations from GHC API. It's also blocking 7867, that I'm currently working on.

comment:12 Changed 6 years ago by Austin Seipp <austin@…>

In bc8a5e14881f0754991390fbc0c37f928887f5eb/ghc:

Persist annotations to interface files (#3725)

Authored-by: Gergely Risko <gergely@risko.hu>
Signed-off-by: Austin Seipp <austin@well-typed.com>

comment:13 Changed 6 years ago by thoughtpolice

Resolution: fixed
Status: patchclosed

Merged, thanks!

comment:14 Changed 6 years ago by Simon Peyton Jones <simonpj@…>

In 7996d8f45866ac8bdf1da189ca6ef1bc38bfe3eb/ghc:

Tidy up the commit for Trac #3725

In particular, the mi_ann_fn field in ModIface was only being used
totally locally in MkIface, so I made it local.

Other than that I just refactored data types slightly,
  * Adding synonym Annotations.AnnPayload for the cryptic Serialized
  * Adding MkIface.IfaceIdExtras to common up some of the identical code
Note: See TracTickets for help on using tickets.