Make calling conventions in template haskell Syntax.hs consistent with those in ghc ForeignCall.hs this impliments #9703 from ghc trac
ClosedPublic

Authored by luite on Oct 19 2014, 7:31 PM.

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.
cmsaperstein updated this revision to Diff 919.Oct 19 2014, 7:31 PM
cmsaperstein retitled this revision from to Make calling conventions in template haskell Syntax.hs consistent with those in ghc ForeignCall.hs this impliments #9703 from ghc trac.
cmsaperstein updated this object.
cmsaperstein edited the test plan for this revision. (Show Details)
cmsaperstein added reviewers: luite, ekmett.
cmsaperstein updated the Trac tickets for this revision.
luite edited edge metadata.Oct 19 2014, 9:14 PM

Conversion of the calling convention is still missing. See compiler/hsSyn/Convert.lhs, cvt_conv

Whoops, I'll fix that.

cmsaperstein updated this revision to Diff 920.Oct 19 2014, 9:34 PM
cmsaperstein edited edge metadata.
  • Fixed non-exhaustive pattern matching error
  • fixed error with call conversions

Updating D353: Make calling conventions in template haskell Syntax.hs consistent with those in ghc ForeignCall.hs

this impliments Trac #9703 from ghc trac

Is this an issue? If so, how do I go about fixing it?

perf/compiler T3064 [stat not good enough] (normal)

Every build of GHC right now is failing that perf test. I think austin or
hvr are going to tweek it later this week. But if not maybe that's a good
second patch !

I think you'll need to update DsMeta.repCCallConv as well. That update should remove the catch-all case at the bottom, to make it harder for people to forget this change in the future. To update DsMeta, you'll probably also need to update Language.Haskell.TH.Lib.

Are there any tests?

Thanks for doing this!

Can you add tests for this and check that unsupported imports give a decent error message? I recall that it's easy to make GHC panic with an invalid C label in a spliced CCall import, which should probably also be fixed.

I'll add the tests, update DsMeta.reCCallConv, and add in the arbitrary string constructor as well. I wont get to it before the weekend, but thanks for the feedback!

ezyang removed a subscriber: ezyang.Oct 27 2014, 2:04 PM
ekmett edited edge metadata.Nov 7 2014, 12:06 PM

So what are we still missing?

luite added a comment.Nov 16 2014, 5:30 PM
In D353#11229, @ekmett wrote:

So what are we still missing?

DsMeta changes and some tests are still missing. I'd like to have this in 7.10, so I'll fix it if Craig doesn't have time.

luite commandeered this revision.Nov 19 2014, 2:58 AM
luite edited reviewers, added: cmsaperstein; removed: luite.

adding missing bits

luite updated this revision to Diff 1540.Nov 19 2014, 3:00 AM
luite edited edge metadata.
  • Add missing calling conventions to DsMeta and Language.Haskell.TH.Lib

Updating D353: Make calling conventions in template haskell Syntax.hs consistent with those in ghc ForeignCall.hs

this impliments Trac #9703 from ghc trac

luite added a comment.Nov 19 2014, 3:11 AM

I've rebased it on the latest master and added the missing DsMeta / Language.Haskell.TH.Lib bits

I don't really know what'd be a good test case, so if anyone's feeling creative..... Producing declarations works, but it's possible to get weird error messages, since the result gets past the first line of checks on the label (this was already the case with the CCall and StdCall conventions).

Bikeshedding call: I haven't used a suffix to the calling convention names exported from Language.Haskell.TH (defined in .TH.Lib) to keep them consistent with the data constructors already there. Of these, prim is probably most likely to clash (it's not a reserved word though, even when GHCForeignImportPrim is active!). Should we add a suffix, renaming them to primCall, cApiCall and javaScriptCall ?

goldfire accepted this revision.Nov 19 2014, 7:49 AM
goldfire edited edge metadata.

LGTM, but a test would be nice. Something simple like this could be tested just by doing a round-trip:

$( do decs <- [d| {- foreign declaration (embarrassingly, I don't know the syntax off hand) -} |]
      runIO $ do
        putStrLn $ pprint decs
        hFlush stdout
      return decs )

and then use a -ddump-xxx flag in the test settings to see that the declaration is accepted as given.

luite added a comment.Nov 19 2014, 4:35 PM

Ok, I'll try to add something. Unfortunately StdCall and JavaScript are platform dependent, StdCall giving a warning when it's not supported and JavaScript always giving an error in vanilla GHC (GHCJS accepts them even when generating native code if there's a JS engine available in the compiled program).

luite added a comment.Nov 19 2014, 5:08 PM

looks like quoting foreign declarations does not yet work:

TH_foreignCallingConventions.hs:10:9:
    Foreign label not (yet) handled by Template Haskell "foo"
luite updated this revision to Diff 1574.Nov 19 2014, 5:42 PM
luite edited edge metadata.
  • add test case for new foreign calling conventions in Template Haskell

Updating D353: Make calling conventions in template haskell Syntax.hs consistent with those in ghc ForeignCall.hs

this impliments Trac #9703 from ghc trac

austin accepted this revision.Nov 19 2014, 6:38 PM
austin edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Nov 19 2014, 6:38 PM
This revision was automatically updated to reflect the committed changes.