Permalink
Browse files

Merge branch 'issue-136'

  • Loading branch information...
ian-ross committed Jun 22, 2015
2 parents 15979f3 + 8c38295 commit 87c705e5dbbb5697a7a724e872a34cdb60555fee
Showing with 570 additions and 189 deletions.
  1. +2 −0 .gitignore
  2. +4 −1 c2hs.cabal
  3. +104 −0 import-handling.md
  4. +9 −0 regression-suite-vm/Vagrantfile
  5. +84 −0 regression-suite-vm/Vagrantfile-full
  6. +20 −0 regression-suite-vm/run-regression
  7. +1 −1 src/C2HS/CHS.hs
  8. +259 −115 src/C2HS/Gen/Bind.hs
  9. +16 −3 src/C2HS/Gen/Monad.hs
  10. +1 −1 src/C2HS/Version.hs
  11. +0 −1 tests/bugs/issue-10/Issue10.chs
  12. +0 −5 tests/bugs/issue-102/Issue102.chs
  13. +1 −2 tests/bugs/issue-115/Issue115.chs
  14. +0 −1 tests/bugs/issue-123/Issue123.chs
  15. +0 −3 tests/bugs/issue-127/Issue127.chs
  16. +1 −5 tests/bugs/issue-130/Issue130.chs
  17. +0 −2 tests/bugs/issue-133/Issue133.chs
  18. +43 −0 tests/bugs/issue-136/Issue136.chs
  19. +5 −0 tests/bugs/issue-136/issue136.c
  20. +11 −0 tests/bugs/issue-136/issue136.h
  21. +0 −1 tests/bugs/issue-15/Issue15.chs
  22. +0 −2 tests/bugs/issue-23/Issue23.chs
  23. +0 −1 tests/bugs/issue-25/Issue25.chs
  24. +0 −4 tests/bugs/issue-31/Issue31.chs
  25. +0 −5 tests/bugs/issue-32/Issue32.chs
  26. +0 −2 tests/bugs/issue-36/Issue36.chs
  27. +0 −2 tests/bugs/issue-38/Issue38.chs
  28. +0 −2 tests/bugs/issue-44/Issue44.chs
  29. +0 −2 tests/bugs/issue-45/Issue45.chs
  30. +0 −3 tests/bugs/issue-46/Issue46.chs
  31. +0 −2 tests/bugs/issue-47/Issue47.chs
  32. +0 −1 tests/bugs/issue-48/Issue48.chs
  33. +0 −3 tests/bugs/issue-54/Issue54.chs
  34. +0 −2 tests/bugs/issue-69/Issue69.chs
  35. +0 −4 tests/bugs/issue-73/Issue73.chs
  36. +0 −4 tests/bugs/issue-75/Issue75.chs
  37. +0 −2 tests/bugs/issue-96/Issue96.chs
  38. +0 −3 tests/bugs/issue-98/Issue98.chs
  39. +5 −4 tests/regression-suite.yaml
  40. +4 −0 tests/test-bugs.hs
View
@@ -151,3 +151,5 @@
/tests/bugs/issue-130/Issue130.hs
/tests/bugs/issue-133/Issue133
/tests/bugs/issue-133/Issue133.hs
+/tests/bugs/issue-136/Issue136.hs
+/tests/bugs/issue-136/Issue136
View
@@ -1,5 +1,5 @@
Name: c2hs
-Version: 0.25.3
+Version: 0.26.1
License: GPL-2
License-File: COPYING
Copyright: Copyright (c) 1999-2007 Manuel M T Chakravarty
@@ -90,7 +90,10 @@ Extra-Source-Files:
tests/bugs/issue-123/*.chs tests/bugs/issue-123/*.h tests/bugs/issue-123/*.c
tests/bugs/issue-127/*.chs tests/bugs/issue-127/*.h tests/bugs/issue-127/*.c
tests/bugs/issue-128/*.chs tests/bugs/issue-128/*.h tests/bugs/issue-128/*.c
+ tests/bugs/issue-130/*.chs tests/bugs/issue-130/*.h tests/bugs/issue-130/*.c
tests/bugs/issue-131/*.chs tests/bugs/issue-131/*.h tests/bugs/issue-131/*.c
+ tests/bugs/issue-133/*.chs tests/bugs/issue-133/*.h
+ tests/bugs/issue-136/*.chs tests/bugs/issue-136/*.h tests/bugs/issue-136/*.c
source-repository head
type: git
View
@@ -0,0 +1,104 @@
+## Potentially breaking changes: import handling
+
+#### The problem
+
+Previous releases of C2HS had an annoying misfeature -- you had to
+manage the imports of Haskell library functions in C2HS-generated code
+yourself. Suppose you had the following code in a `.chs` file:
+
+``` haskell
+#include "issue44.h"
+
+{#pointer *foo as ^ foreign newtype#}
+```
+
+where the contents of the `issue44.h` header are:
+
+``` c
+typedef struct { int a; } foo;
+```
+
+Running C2HS would then generate the following Haskell code:
+
+``` haskell
+newtype Foo = Foo (ForeignPtr (Foo))
+withFoo :: Foo -> (Ptr Foo -> IO b) -> IO b
+withFoo (Foo fptr) = withForeignPtr fptr
+```
+
+Note the use of the names `ForeignPtr`, `Ptr` and `withForeignPtr`.
+These come from the Haskell library modules `Foreign.Ptr` and
+`Foreign.ForeignPtr`, but C2HS didn't generate any `import`
+declarations to make these modules accessible. This meant that there
+would normally be a bit of back and forth when writing C2HS code:
+write your bindings, run C2HS, try compiling with GHC, have the
+compile fail because of missing imports, add the imports to your
+`.chs` file and repeat. Kind of annoying.
+
+As well as being annoying, the lack of import declaration generation
+meant that it was sometimes impossible to make internal changes to the
+way that C2HS binds to C functions without breaking existing user
+code. The example that finally drove me to try to fix this was issue
+#130 (https://github.com/haskell/c2hs/issues/130) that required a
+change that would lead to most C2HS code now needing to import
+`unsafePerformIO`. It didn't seem like a good idea to push a change
+like that (that would break more or less all C2HS code out there!)
+without fixing the import problem (so that the change for issue #130
+could happen transparently to all existing working C2HS code).
+
+
+#### The solution
+
+The solution I ended up with is pretty simple, but I think it's
+robust. For the example above, C2HS now generates the following
+Haskell code:
+
+``` haskell
+import qualified Foreign.ForeignPtr as C2HSImp
+import qualified Foreign.Ptr as C2HSImp
+
+newtype Foo = Foo (C2HSImp.ForeignPtr (Foo))
+withFoo :: Foo -> (C2HSImp.Ptr Foo -> IO b) -> IO b
+withFoo (Foo fptr) = C2HSImp.withForeignPtr fptr
+```
+
+All library symbols needed to generate Haskell binding code are now
+qualified under the name `C2HSImp` and the relevant library modules
+are imported qualified as `C2HSImp`.
+
+The end result of this is that you still need to import modules only
+for names that you explicitly use (so if you use `alloca` in an input
+marshaller, you need to import `Foreign.Marshal.Alloc`). All external
+names that C2HS uses in code that it generates should be imported
+automatically.
+
+
+#### Potential complaints
+
+1. Modules compiled with `-Werror` may now fail because of unused
+ import warnings. This was something I had to deal with for most of
+ the C2HS test cases (since they all imported the required library
+ modules and they're mostly compiled with `-Werror`), but since the
+ community consensus seems to be that `-Werror` shouldn't be used in
+ released code, I think it's reasonable to allow the possibility of
+ this kind of breakage.
+
+2. It's possible that the code I wrote for deciding where to put the
+ extra import declarations isn't quite perfect. I tried a couple of
+ different solutions, but ended up with a hand-made "find the first
+ safe place to add imports" function that relies quite heavily on
+ the details of C2HS's CHS file parser. I did try a solution based
+ on `haskell-src-exts`, but this didn't work very well, because
+ `haskell-src-exts` doesn't support all available GHC extensions and
+ I would have needed some mechanism to propagate extension
+ information from Cabal files to C2HS to make the parsing work.
+
+These changes have been tested reasonably extensively -- all of the
+core C2HS tests pass, and the following packages are known to work
+(they're all in the regression suite): abcBridge, alsa-mixer, cuda,
+cufft, gnome-keyring, gnuidn, haskell-mpi, hnetcdf, hpuz, hsndfile,
+hsshellscript, igraph, libssh2.
+
+I'll be adding more packages to the regression suite, but if there's a
+package you're particularly concerned about that's not on this list,
+let me know.
@@ -0,0 +1,9 @@
+Vagrant.configure("2") do |config|
+ config.vm.box = "c2hs-regression-suite.box"
+
+ config.vm.provider :virtualbox do |vb|
+ vb.gui = false
+ vb.customize ["modifyvm", :id, "--memory", "2048"]
+ vb.customize ["modifyvm", :id, "--cpus", "2"]
+ end
+end
@@ -0,0 +1,84 @@
+$script = <<SCRIPT
+
+set -e
+
+# Initial APT package installation
+
+sed -i -e 's/us.archive.ubuntu.com/gb.archive.ubuntu.com/g' /etc/apt/sources.list
+sed -i -e '/trusty multiverse/s/^# //' -e '/trusty-updates multiverse/s/^# //' /etc/apt/sources.list
+
+apt-get update
+apt-get install -y python-pip
+pip install awscli
+
+mkdir cuda-packages
+aws s3 sync s3://cuda-packages ./cuda-packages
+dpkg -i ./cuda-packages/cuda-repo-ubuntu1204_6.5-14_amd64.deb
+/bin/rm ./cuda-packages/cuda-repo-ubuntu1204_6.5-14_amd64.deb
+mv ./cuda-packages/*.deb /var/cache/apt/archives
+/bin/rm -fr ./cuda-packages
+
+apt-add-repository -y ppa:igraph/ppa
+apt-get update
+
+PACKAGES="alex build-essential git happy libatlas-base-dev libgmp-dev
+ liblapack-dev libnetcdf-dev unzip zlib1g-dev libopencv-dev
+ libcv-dev libhighgui-dev libgnome-keyring-dev libgsl0-dev
+ libsndfile1-dev libqtscript4-core libqt4-declarative
+ libigraph0-dev acpid consolekit dkms lib32gcc1 libc-bin
+ libc-dev-bin libc6 libc6-dev libc6-i386 libck-connector0
+ libpam-ck-connector libpolkit-agent-1-0
+ libpolkit-backend-1-0 libpolkit-gobject-1-0 libvdpau1
+ libxmu-dev libxmu-headers policykit-1 policykit-1-gnome
+ python-xkit screen-resolution-extra cuda libasound2-dev
+ libidn11-dev libopenmpi-dev libssh2-1-dev"
+
+apt-get install -y $PACKAGES
+
+cd /usr/lib/x86_64-linux-gnu
+ln -s libgmp.so.10 libgmp.so.3
+cd /root
+
+
+# GHC and Cabal installation
+wget https://www.haskell.org/ghc/dist/7.8.3/ghc-7.8.3-x86_64-unknown-linux-deb7.tar.xz
+tar xJf ghc-7.8.3-x86_64-unknown-linux-deb7.tar.xz
+cd ghc-7.8.3
+./configure
+make install
+cd ..
+/bin/rm -fr ghc-7.8.3*
+
+wget https://www.haskell.org/cabal/release/cabal-install-1.20.0.3/cabal-install-1.20.0.3.tar.gz
+tar xzf cabal-install-1.20.0.3.tar.gz
+cd cabal-install-1.20.0.3
+./bootstrap.sh
+cd ..
+/bin/rm -fr cabal-install-1.20.0.3*
+
+if [ ! -f /usr/bin/cabal-1.20 ]; then
+ cp /root/.cabal/bin/cabal /usr/bin/cabal-1.20
+ (cd /usr/bin/ && rm -rf cabal && ln -s cabal-1.20 cabal)
+fi
+
+echo "export PATH=\$HOME/.cabal/bin:\$PATH" >> /etc/bash.bashrc
+echo "export PATH=/usr/local/cuda-6.5/bin:\$PATH" >> /etc/bash.bashrc
+
+su vagrant -c 'cabal update'
+
+SCRIPT
+
+Vagrant.configure("2") do |config|
+ config.vm.box = "trusty-server-cloudimg-amd64-vagrant-disk1.box"
+ config.vm.box_url = "https://cloud-images.ubuntu.com/vagrant/trusty/current/trusty-server-cloudimg-amd64-vagrant-disk1.box"
+
+ config.vm.synced_folder "..", "/home/vagrant/c2hs"
+
+ config.vm.provider :virtualbox do |vb|
+ vb.gui = false
+ vb.customize ["modifyvm", :id, "--memory", "2048"]
+ vb.customize ["modifyvm", :id, "--cpus", "2"]
+ end
+
+ config.vm.provision "shell", inline: $script
+end
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+branch=${1:-master}
+
+vagrant up
+vagrant ssh <<EOF
+/bin/rm -fr c2hs
+export LD_LIBRARY_PATH=/usr/local/cuda-6.5/lib64:$LD_LIBRARY_PATH
+git clone https://github.com/haskell/c2hs.git
+cd c2hs
+git checkout $branch
+cabal update
+cabal install --only-dep --enable-tests -fregression
+cabal install
+cabal configure --enable-tests -fregression
+cabal build && cabal test
+export C2HS_REGRESSION_SUITE=1
+./dist/build/regression-suite/regression-suite
+EOF
+vagrant destroy -f
View
@@ -904,7 +904,7 @@ parseCHSModule :: Position -> String -> CST s CHSModule
parseCHSModule pos cs = do
toks <- lexCHS cs pos
frags <- parseFrags toks
- return (CHSModule frags)
+ return $ CHSModule frags
-- | parsing of code fragments
--
Oops, something went wrong.

0 comments on commit 87c705e

Please sign in to comment.