Handle annotations in .hsig files (fixes #10621)
Needs RevisionPublic

Authored by spinda on Jul 22 2015, 6:16 AM.

Details

Reviewers
ezyang
bgamari
austin
Trac Issues
#10621
Summary

Previously they were thrown away during typechecking. Now they are
passed through as they are for normal hs source files, and stored in and
loaded from interface files.

This patch consists of (1) passing .hsig annotations through the
typechecker, (2) adding support for annotations in .hsig interface
files, and (3) adding a test case.

Test Plan

validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
hsig-annotations
Lint
Lint WarningsExcuse: Matching formatting of surrounding lines
SeverityLocationCodeMessage
Warningcompiler/iface/LoadIface.hs:490TXT3Line Too Long
Unit
No Unit Test Coverage
Build Status
Buildable 4978
Build 5112: GHC Patch Validation (amd64/Linux)
Build 5111: arc lint + arc unit
spinda retitled this revision from to Handle annotations in .hsig files (fixes #10621).Jul 22 2015, 6:16 AM
spinda updated this object.
spinda edited the test plan for this revision. (Show Details)
spinda added a reviewer: austin.
spinda updated the Trac tickets for this revision.
bgamari requested changes to this revision.Jul 22 2015, 9:49 AM

It looks like this might need to be rebased.

This revision now requires changes to proceed.Jul 22 2015, 9:49 AM
spinda updated this revision to Diff 3644.Jul 22 2015, 3:40 PM
  • Rebase

One question inline. Otherwise looks good to me.

compiler/main/DynFlags.hs
3675

I'm not sure I see how this change fits with the theme of this Diff. Does Backpack add _ as a valid character in a module name? Perhaps this should be in a separate commit?

thomie added a subscriber: ezyang.

Suggestion: @ezyang and @spinda add eachother as reviewers.

ezyang requested changes to this revision.Jul 25 2015, 7:43 PM

I think lookupIfaceTop should do the right thing. Let me know if it does not.

compiler/iface/TcIface.hs
681

Threading sig-of here should not be necessary; lookupIfaceTop will use if_mod, which should be set to the sig-of.

compiler/main/DynFlags.hs
3675

This is an unrelated bugfix; modules always allowed underscores but the parser here didn't handle it.

This revision now requires changes to proceed.Jul 25 2015, 7:43 PM

@ezyang Is this still relevant since the changes you've been making to Backpack?

compiler/iface/TcIface.hs
681

I remember having to change this because of a failure in one of the test cases I added. I'll go back and check again, though.

compiler/main/DynFlags.hs
3675

I fixed this here because I ran into the issue when writing the test cases. Should I split it into another commit?

ezyang added a comment.Sep 3 2015, 4:17 PM

Yep, this is still relevant, because this patch also handles annotations in hs-boot files.

compiler/main/DynFlags.hs
3675

I just went ahead and pushed this fix, if you rebase it should go away!

spinda added a comment.Sep 3 2015, 4:47 PM

Okay, I'll get this up to date and make the requested changes, then.

What is the status of this?

ezyang added a comment.Apr 6 2017, 8:42 PM

It basically needs to be rewritten for the new Backpack stuff.

austin resigned from this revision.Nov 6 2017, 10:09 PM