From: Duncan C. <dun...@wo...> - 2005-08-18 17:04:38
|
On Thu, 2005-08-18 at 15:45 +0100, Axel Simon wrote: > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > which is essentially the same as the with* function, but used > > automatically and inlined at the call site. > > It also doesn't pollute the name space. This is more severe that it may > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > about the c2hs type declarations in the module. If we were to switch to > with* functions, we would also have to add these with* functions to the > export list, depending on weather we use the type somewhere else or not. > Furthermore we should then hide all these with* functions in Gtk.hs. > This imposes quite a significant maintenance problem with no apparent > benefit. I'm not sure that it would be much different. With our solution we have to export the newtype constructor name to other binding modules and make sure that they do not escape from the top level Gtk module. We do this by putting all the {# pointer ... #} hooks in one module which is not directly exported by the top level module. I think this would be the same if this Gtk.Types module exported with* functions instead of the constructor names themselves. It would be nice if c2hs used the with* functions automatically like we have in our c2hs fork (since for one thing it makes it easier for people to experiment and switch between foriegn/newtype/foriegn+newtype/raw), but we could change our code to use the with* functions reasonably easily by changing our code generator. Duncan |
From: Duncan C. <dun...@wo...> - 2005-08-18 17:14:04
|
I may have replied to this already. Can't quite remember now. Last week was all a blur. On Sun, 2005-08-07 at 22:59 +1000, Manuel M T Chakravarty wrote: > > BTW, we've still got another patch that's we probably need before we can > > use the mainline c2hs again. However I don't understand it! I'll have to > > get Axel to look into that one. :-) It's something in the GenBind > > module. As I recall it was something to do with foreign pointers. > > How big is the change? I would ask Axel to explain the problem and his solution/patch but he's rather busy with other work. It'll be easeir for me to describe the problem/feature than the implemntation. So apparently newtype's can be passed directly as FFI args. I didn't realise that. eg: {#pointer *cairo_t as Cairo newtype#} which becomes newtype Cairo = Cairo (Ptr (Cairo)) and then for a call we get this decl: foreign import ccall safe "cairo_create" create :: ((Surface) -> (IO (Cairo))) rather than Ptr Cairo. Like I said, I didn't realsie that this worked, but it's convenient that it does. However it doesn't work for ForeignPtr. Since they need to be unwrapped. The latest c2hs generates with* functions for these: {#pointer *cairo_t as Cairo foreign newtype#} newtype Cairo = Cairo (ForeignPtr (Cairo)) withCairo (Cairo fptr) = withForeignPtr fptr So I guess the user is supposed to use this withCairo function to unwrap the newtype and the ForeignPtr. The current version of c2hs in the Gtk2Hs tree takes a different approach. This is a rather old patch whihc I think predates the with* functions that c2hs builds. So it tries to make it more transparent by doing the unwrapping automatically at a call site: So this buttonPressed :: ButtonClass self => self -> IO () buttonPressed self = {# call button_pressed #} (toButton self) becomes: buttonPressed :: ButtonClass self => self -> IO () buttonPressed self = (\(Button arg1) -> withForeignPtr arg1 $ \argPtr1 ->gtk_button_pressed argPtr1) (toButton self) which is essentially the same as the with* function, but used automatically and inlined at the call site. Now since we didn't realise that newtypes can be used directly in FFI calls the patch also does the newtype unwrapping for non-ForeignPtr {#pointer _ newtype#} types too. This can obviously be removed. So I suppose this is a feature request: automatic application of the with* functions. Otherwise I suppose I could go modify our code generator to produce .chs modules that use the with* functions that c2hs generates, rather than relying on c2hs to apply them automatically. Axel, any opinion on this? > Not including any .h in foreign imports is not a portable way of using > the FFI. Sorry, I don't think I was clear enough. We use the -#include on the command line (and in the package.conf file) to specify the header files to include. So we are compiling with the correct .h files specified. So the point is that we want to use the .h files specified in the package.conf file (or on the commandline during the build itself) rather than lots of little .h files that just #include the main .h file. So for example we build the modules like this: ghc -c gtk/Graphics/UI/Gtk/Buttons/Button.hs -o gtk/Graphics/UI/Gtk/Buttons/Button.o -fffi -iglib:gtk -package-name gtk '-#include<gtk/gtk.h>' -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -DXTHREADS -D_REENTRANT -DXUSE_MTSAFE_API -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/freetype2 -I/usr/include/atk-1.0 So we specify the '-#include<gtk/gtk.h>' and all the -I dirs on the command line. And the gtk.package.conf file contains: extra-libraries: "gtk-x11-2.0","gdk-x11-2.0","atk-1.0","pangoxft-1.0","pangox-1.0","pango-1.0","gdk_pixbuf-2.0","m","gmodule-2.0","dl" include-dirs: "/usr/include/gtk-2.0", "/usr/lib/gtk-2.0/include", "/usr/include/pango-1.0", "/usr/include/freetype2", "/usr/include/atk-1.0" includes: gtk/gtk.h So that importing modules will get the correct include (and search path). So we want to generate import decls like this: foreign import ccall safe "gtk_button_pressed" gtk_button_pressed :: ((Ptr Button) -> (IO ())) instead of like this: foreign import ccall safe "gtk/Graphics/UI/Gtk/Buttons/Button.h gtk_button_pressed" gtk_button_pressed :: ((Ptr Button) -> (IO ())) where gtk/Graphics/UI/Gtk/Buttons/Button.h contains nothing but: #include <gtk/gtk.h> Since that way we do not need to distribute lots of little .h files. We directly include the apprpriate system header. Another reason for this is that ghc corrently does not inline FFI calls which are declared in the latter form above (since it would also have to propogate the .h files across module boundaries, and it currently doesn't do that). See: http://haskell.org/ghc/docs/latest/html/users_guide/sec-ffi-ghc.html#glasgow-foreign-headers > The only feasible "optimisation" for c2hs seems to be that if > it generates a "foo.h" with only > > #include "lib.h" We currently do not put the #include "lib.h" into the .chs module since we already have to pre-process the .chs module with cpp. That's why we specify it on the c2hs and ghc command line. > then it could drop "foo.h" and put "lib.h" into the foreign import > declaration. That'd be a reasonable, generally useful feature, which > I'd be happy to include. I'd prefer not applying this "optimisation" by > default, though, but only when requested via a command line option. > That is as some people may rely on the .h generated by c2hs in their > packages. > > Is that a reasonable compromise? It's no problem if we have to specify and extra option to get this behaviour. > I am happy to rely on anything that is in the hierarchical libraries of > GHC 6.2 *and* of nhc98 and Hugs. I'd like to keep the code base > portable. AFAIK, Data.Map is portable. Sadly, Data.Map is currently only in GHC 6.4 and the latest hugs release. There is a wrapper module floating around somewhere that implements the Data.Map interface on the Data.FiniteMap implementation that comes with earlier versions of GHC, nhc and hugs. But that's a less important issue. Duncan |
From: Axel S. <A....@ke...> - 2005-08-18 23:49:18
|
Duncan, Manuel, this is a bit of waffle, I tried to reiterate the reasons why things are the way they are. Here's my (longer) story. On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > I may have replied to this already. Can't quite remember now. Last week > was all a blur. > > On Sun, 2005-08-07 at 22:59 +1000, Manuel M T Chakravarty wrote: > > > BTW, we've still got another patch that's we probably need before we can > > > use the mainline c2hs again. However I don't understand it! I'll have to > > > get Axel to look into that one. :-) It's something in the GenBind > > > module. As I recall it was something to do with foreign pointers. > > > > How big is the change? > > I would ask Axel to explain the problem and his solution/patch but he's > rather busy with other work. > > It'll be easeir for me to describe the problem/feature than the > implemntation. > > So apparently newtype's can be passed directly as FFI args. I didn't > realise that. eg: > > {#pointer *cairo_t as Cairo newtype#} > > which becomes > > newtype Cairo = Cairo (Ptr (Cairo)) > > and then for a call we get this decl: > > foreign import ccall safe "cairo_create" > create :: ((Surface) -> (IO (Cairo))) > > rather than Ptr Cairo. I knew this worked once, but also got a bit confused. The FFI appendix stopped ForeignPtrs to be marshaled automatically: "The argument types ati produced by fatype must be marshallable foreign types; that is, each ati is either (1) a basic foreign type or (2) a type synonym or renamed datatype of a marshallable foreign type." A "renamed datatype" is a newtype declaration, so these still work. Before 6.00, ForeignPtrs and newtype-wrapped ForeignPtrs were accepted as well. Hence I could have kept the constructors around plain newtype declarations. However, I wasn't over zealous in stripping these constructors off. There is a reason for this, see below. > Like I said, I didn't realsie that this worked, but it's convenient that > it does. However it doesn't work for ForeignPtr. Since they need to be > unwrapped. > > The latest c2hs generates with* functions for these: > > {#pointer *cairo_t as Cairo foreign newtype#} > > newtype Cairo = Cairo (ForeignPtr (Cairo)) > withCairo (Cairo fptr) = withForeignPtr fptr > > So I guess the user is supposed to use this withCairo function to unwrap > the newtype and the ForeignPtr. > > The current version of c2hs in the Gtk2Hs tree takes a different > approach. This is a rather old patch whihc I think predates the with* > functions that c2hs builds. Yes, I think it does pre-date this with* functions. > So it tries to make it more transparent by doing the unwrapping > automatically at a call site: > > So this > > buttonPressed :: ButtonClass self => self -> IO () > buttonPressed self = > {# call button_pressed #} > (toButton self) > > becomes: > > buttonPressed :: ButtonClass self => self -> IO () > buttonPressed self = > (\(Button arg1) -> withForeignPtr arg1 $ \argPtr1 ->gtk_button_pressed argPtr1) > (toButton self) > > which is essentially the same as the with* function, but used > automatically and inlined at the call site. It also doesn't pollute the name space. This is more severe that it may seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know about the c2hs type declarations in the module. If we were to switch to with* functions, we would also have to add these with* functions to the export list, depending on weather we use the type somewhere else or not. Furthermore we should then hide all these with* functions in Gtk.hs. This imposes quite a significant maintenance problem with no apparent benefit. > Now since we didn't realise that newtypes can be used directly in FFI > calls the patch also does the newtype unwrapping for non-ForeignPtr > {#pointer _ newtype#} types too. This can obviously be removed. This is where the problems crept up: If you have a newtype wrapping a ForeignPtr, you need to stip the newtype constructor before you can use withForeignPtr on it. OTOH, if you have a normal pointer wrapped in a newtype, you can simply pass the unstripped value to the function. Since you need the code to strip the constructors for ForeignPtrs, it is actually easier to do it for simple newtypes around Ptrs as well. At least emitting the "foreign import" stubs is the same for both, newtype- wrapped Ptrs and ForeignPtrs. The reason why Manuel didn't want to apply my patch is a different one: I only fixed c2hs half-heartedly. IIRC, our version copes with all possible newtype wrapped pointers, but not with normal pointers for which c2hs can generate type synonyms. In GBMonad.hs, I changed type PointerMap = FiniteMap (Bool, Ident) (CHSPtrType, String) to type PointerMap = FiniteMap (Bool, Ident) HsPtrRep type HsPtrRep = (Bool, CHSPtrType, Maybe String, String) . There is a comment explaining the fields of HsPtrRep in http://cvs.sourceforge.net/viewcvs.py/gtk2hs/gtk2hs/tools/c2hs/gen/GBMonad.hs?rev=1.1&view=markup (I can't seem to find the point where this was actually added. We moved this file from c2hs/gen/ to tools/c2hs/c2hs/gen to tools/c2hs/gen but there's no trace of this modification.) As far as I know, this map is not sufficiently expressive to indicate that a certain type may actually not newtype-wrapped but just type- wrapped. In these cases no constructor may be stripped off. To fix this we need: a) somebody to understand HsPtrRep again b) somebody to go through the whole of GenBind and fix the code emitting phase. I didn't do this back then because I changed HsPtrRep several times and couldn't face to do it again. Axel. |
From: Manuel M T C. <ch...@cs...> - 2005-08-21 12:57:52
|
Axel Simon: > Duncan, Manuel, > > this is a bit of waffle, I tried to reiterate the reasons why things are > the way they are. Here's my (longer) story. > > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > I may have replied to this already. Can't quite remember now. Last week > > was all a blur. > > > > On Sun, 2005-08-07 at 22:59 +1000, Manuel M T Chakravarty wrote: > > > > BTW, we've still got another patch that's we probably need before we can > > > > use the mainline c2hs again. However I don't understand it! I'll have to > > > > get Axel to look into that one. :-) It's something in the GenBind > > > > module. As I recall it was something to do with foreign pointers. > > > > > > How big is the change? > > > > I would ask Axel to explain the problem and his solution/patch but he's > > rather busy with other work. > > > > It'll be easeir for me to describe the problem/feature than the > > implemntation. > > > > So apparently newtype's can be passed directly as FFI args. I didn't > > realise that. eg: > > > > {#pointer *cairo_t as Cairo newtype#} > > > > which becomes > > > > newtype Cairo = Cairo (Ptr (Cairo)) > > > > and then for a call we get this decl: > > > > foreign import ccall safe "cairo_create" > > create :: ((Surface) -> (IO (Cairo))) > > > > rather than Ptr Cairo. > > I knew this worked once, but also got a bit confused. The FFI appendix > stopped ForeignPtrs to be marshaled automatically: > > "The argument types ati produced by fatype must be marshallable foreign > types; that is, each ati is either (1) a basic foreign type or (2) a > type synonym or renamed datatype of a marshallable foreign type." > > A "renamed datatype" is a newtype declaration, so these still work. > Before 6.00, ForeignPtrs and newtype-wrapped ForeignPtrs were accepted > as well. Yes, but that had a serious problem for result types, as GHC cannot "guess" which finalizer to attach to a ForeignPtr marshaled from C to Haskell land. > Hence I could have kept the constructors around plain newtype > declarations. However, I wasn't over zealous in stripping these > constructors off. There is a reason for this, see below. > > > Like I said, I didn't realsie that this worked, but it's convenient that > > it does. However it doesn't work for ForeignPtr. Since they need to be > > unwrapped. > > > > The latest c2hs generates with* functions for these: > > > > {#pointer *cairo_t as Cairo foreign newtype#} > > > > newtype Cairo = Cairo (ForeignPtr (Cairo)) > > withCairo (Cairo fptr) = withForeignPtr fptr > > > > So I guess the user is supposed to use this withCairo function to unwrap > > the newtype and the ForeignPtr. > > > > The current version of c2hs in the Gtk2Hs tree takes a different > > approach. This is a rather old patch whihc I think predates the with* > > functions that c2hs builds. > > Yes, I think it does pre-date this with* functions. > > > So it tries to make it more transparent by doing the unwrapping > > automatically at a call site: > > > > So this > > > > buttonPressed :: ButtonClass self => self -> IO () > > buttonPressed self = > > {# call button_pressed #} > > (toButton self) > > > > becomes: > > > > buttonPressed :: ButtonClass self => self -> IO () > > buttonPressed self = > > (\(Button arg1) -> withForeignPtr arg1 $ \argPtr1 ->gtk_button_pressed argPtr1) > > (toButton self) > > > > which is essentially the same as the with* function, but used > > automatically and inlined at the call site. > > It also doesn't pollute the name space. This is more severe that it may > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > about the c2hs type declarations in the module. If we were to switch to > with* functions, we would also have to add these with* functions to the > export list, depending on weather we use the type somewhere else or not. > Furthermore we should then hide all these with* functions in Gtk.hs. > This imposes quite a significant maintenance problem with no apparent > benefit. Yes, that's a good point, but what do you do when a ForeignPtr is returned as a result? What finalizer do you attach? > > Now since we didn't realise that newtypes can be used directly in FFI > > calls the patch also does the newtype unwrapping for non-ForeignPtr > > {#pointer _ newtype#} types too. This can obviously be removed. > > This is where the problems crept up: If you have a newtype wrapping a > ForeignPtr, you need to stip the newtype constructor before you can use > withForeignPtr on it. OTOH, if you have a normal pointer wrapped in a > newtype, you can simply pass the unstripped value to the function. Since > you need the code to strip the constructors for ForeignPtrs, it is > actually easier to do it for simple newtypes around Ptrs as well. At > least emitting the "foreign import" stubs is the same for both, newtype- > wrapped Ptrs and ForeignPtrs. > > The reason why Manuel didn't want to apply my patch is a different one: > I only fixed c2hs half-heartedly. IIRC, our version copes with all > possible newtype wrapped pointers, but not with normal pointers for > which c2hs can generate type synonyms. In GBMonad.hs, I changed > > type PointerMap = FiniteMap (Bool, Ident) (CHSPtrType, String) > > to > > type PointerMap = FiniteMap (Bool, Ident) HsPtrRep > type HsPtrRep = (Bool, CHSPtrType, Maybe String, String) > > . There is a comment explaining the fields of HsPtrRep in > http://cvs.sourceforge.net/viewcvs.py/gtk2hs/gtk2hs/tools/c2hs/gen/GBMonad.hs?rev=1.1&view=markup > > (I can't seem to find the point where this was actually added. We moved > this file from c2hs/gen/ to tools/c2hs/c2hs/gen to tools/c2hs/gen but > there's no trace of this modification.) > > As far as I know, this map is not sufficiently expressive to indicate > that a certain type may actually not newtype-wrapped but just type- > wrapped. That's indeed a problem. I wonder why you implemented this feature in the way you did. Sticking with the above example, why not generate the following? buttonPressed :: ButtonClass self => self -> IO () buttonPressed self = button_pressed_wrapper (toButton self) button_pressed_wrapper (Button arg1) = withForeignPtr arg1 $ \argPtr1 -> gtk_button_pressed argPtr1 The wrapper could go into the bucket for delayed code, in the same way the foreign declaration does. Manuel |
From: Manuel M T C. <ch...@cs...> - 2005-08-21 12:43:12
|
[This reply only address the second issue, to keep the discussion better structured.] Duncan Coutts: > > Not including any .h in foreign imports is not a portable way of using > > the FFI. > > Sorry, I don't think I was clear enough. > > We use the -#include on the command line (and in the package.conf file) > to specify the header files to include. So we are compiling with the > correct .h files specified. So the point is that we want to use the .h > files specified in the package.conf file (or on the commandline during > the build itself) rather than lots of little .h files that just #include > the main .h file. > > So for example we build the modules like this: > > ghc -c gtk/Graphics/UI/Gtk/Buttons/Button.hs -o > gtk/Graphics/UI/Gtk/Buttons/Button.o -fffi -iglib:gtk -package-name gtk > '-#include<gtk/gtk.h>' -I/usr/include/glib-2.0 > -I/usr/lib/glib-2.0/include -DXTHREADS -D_REENTRANT -DXUSE_MTSAFE_API > -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include > -I/usr/include/pango-1.0 -I/usr/include/freetype2 -I/usr/include/atk-1.0 > > So we specify the '-#include<gtk/gtk.h>' and all the -I dirs on the > command line. And the gtk.package.conf file contains: Well, the problem is that -#include is GHC specific. > extra-libraries: "gtk-x11-2.0","gdk-x11-2.0","atk-1.0","pangoxft-1.0","pangox-1.0","pango-1.0","gdk_pixbuf-2.0","m","gmodule-2.0","dl" > include-dirs: "/usr/include/gtk-2.0", "/usr/lib/gtk-2.0/include", "/usr/include/pango-1.0", "/usr/include/freetype2", "/usr/include/atk-1.0" > includes: gtk/gtk.h > > So that importing modules will get the correct include (and search > path). > > So we want to generate import decls like this: > > foreign import ccall safe "gtk_button_pressed" > gtk_button_pressed :: ((Ptr Button) -> (IO ())) > > instead of like this: > > foreign import ccall safe "gtk/Graphics/UI/Gtk/Buttons/Button.h gtk_button_pressed" > gtk_button_pressed :: ((Ptr Button) -> (IO ())) > > where gtk/Graphics/UI/Gtk/Buttons/Button.h contains nothing but: > #include <gtk/gtk.h> > > Since that way we do not need to distribute lots of little .h files. We > directly include the apprpriate system header. I understand that you want to get rid of the many .h files. My proposal is to do that by generating foreign import ccall safe "gtk/gtk.h gtk_button_pressed" gtk_button_pressed :: ((Ptr Button) -> (IO ())) This gets rid of the .h file, but is still portable. Obviously, this can only be done in cases, where the "little .h file" contains only a single #include. And it should only be done when a special command line option is given, but that should not be a problem for you. > Another reason for this is that ghc corrently does not inline FFI calls > which are declared in the latter form above (since it would also have to > propogate the .h files across module boundaries, and it currently > doesn't do that). > > See: > http://haskell.org/ghc/docs/latest/html/users_guide/sec-ffi-ghc.html#glasgow-foreign-headers > > > The only feasible "optimisation" for c2hs seems to be that if > > it generates a "foo.h" with only > > > > #include "lib.h" > > We currently do not put the #include "lib.h" into the .chs module since > we already have to pre-process the .chs module with cpp. That's why we > specify it on the c2hs and ghc command line. Why do you do that? (The solution that I propose above will still work when you specify the .h on the c2hs command line.) Manuel |
From: Axel S. <A....@ke...> - 2005-08-21 14:33:35
|
I can quickly reply to this: On Sun, 2005-08-21 at 22:44 +1000, Manuel M T Chakravarty wrote: > > We currently do not put the #include "lib.h" into the .chs module > since > > we already have to pre-process the .chs module with cpp. That's why > we > > specify it on the c2hs and ghc command line. > > Why do you do that? > Because if we'd use c2hs' conditional compilation, we couldn't use the pre-processed header of gtk.h. Even though we now have a reasonably fast C front end, it wouldn't be nice to give up on this advantage. I noticed that Gtk+, which is 80Mb of C code, needs a similar amount to compile than Gtk2Hs. Axel. |
From: Manuel M T C. <ch...@cs...> - 2005-08-22 06:37:47
|
Axel Simon: > I can quickly reply to this: > > On Sun, 2005-08-21 at 22:44 +1000, Manuel M T Chakravarty wrote: > > > We currently do not put the #include "lib.h" into the .chs module > > since > > > we already have to pre-process the .chs module with cpp. That's why > > we > > > specify it on the c2hs and ghc command line. > > > > Why do you do that? > > > Because if we'd use c2hs' conditional compilation, we couldn't use the > pre-processed header of gtk.h. Even though we now have a reasonably fast > C front end, it wouldn't be nice to give up on this advantage. I see. Does this make a big difference? (Doesn't really matter, as we can still use the scheme I proposed even with pre-cpp'ed headers.) > I noticed > that Gtk+, which is 80Mb of C code, needs a similar amount to compile > than Gtk2Hs. After your generator, c2hs, and GHC are have translated all of Gtk2Hs to C code, its probably not much less than 80MB either. Manuel |
From: Duncan C. <dun...@wo...> - 2005-08-21 14:50:14
|
On Sun, 2005-08-21 at 15:32 +0100, Axel Simon wrote: > I can quickly reply to this: > > On Sun, 2005-08-21 at 22:44 +1000, Manuel M T Chakravarty wrote: > > > We currently do not put the #include "lib.h" into the .chs module > > since > > > we already have to pre-process the .chs module with cpp. That's why > > we > > > specify it on the c2hs and ghc command line. > > > > Why do you do that? > > > Because if we'd use c2hs' conditional compilation, we couldn't use the > pre-processed header of gtk.h. Even though we now have a reasonably fast > C front end, it wouldn't be nice to give up on this advantage. I noticed > that Gtk+, which is 80Mb of C code, needs a similar amount to compile > than Gtk2Hs. Actually I'm not sure that the overall build time would be much different between an external c2hs that doesn't support the precomp feature and a bundled c2hs with the feature. On my machine, the time to process a .chs module is roughly 3 seconds without using the precompiled headers and about 1 second when using it. However if we take into account the time to build c2hs, then the overall time works out to be not that different. Duncan |
From: Manuel M T C. <ch...@cs...> - 2005-08-22 06:48:56
|
Duncan Coutts: > On my machine, the time to process a .chs module is roughly 3 seconds > without using the precompiled headers and about 1 second when using it. > However if we take into account the time to build c2hs, then the overall > time works out to be not that different. So, c2hs takes half the time of cpp to do its job? That's not bad at all! Manuel |
From: Duncan C. <dun...@wo...> - 2005-08-22 09:08:04
|
On Mon, 2005-08-22 at 16:49 +1000, Manuel M T Chakravarty wrote: > Duncan Coutts: > > On my machine, the time to process a .chs module is roughly 3 seconds > > without using the precompiled headers and about 1 second when using it. > > However if we take into account the time to build c2hs, then the overall > > time works out to be not that different. > > So, c2hs takes half the time of cpp to do its job? That's not bad at > all! Sadly No. The times were including cpp which takes something like 0.1 sec to process the gtk headers and write them into the .i file. It takes 2 sec or so to parse the .i file (and rather longer to serialise it into our precomp header file for reasons that we still do not properly understand) So I hope that you might still accept a few performance patches here and there. I was thinking of borrowing some techniques from the ghc lexer/parser which is very fast. It does things like lexing directly from a memory mapped file rather than going via a lazy string. That also allows them to use packed strings with zero copy (ie they leave the actual string data in the memory mapped file!) Some of the c2hs data structures could be slightly more compact and/or stricter in places. Duncan |
From: Manuel M T C. <ch...@cs...> - 2005-08-23 13:47:25
|
Duncan Coutts: > On Mon, 2005-08-22 at 16:49 +1000, Manuel M T Chakravarty wrote: > > Duncan Coutts: > > > On my machine, the time to process a .chs module is roughly 3 seconds > > > without using the precompiled headers and about 1 second when using it. > > > However if we take into account the time to build c2hs, then the overall > > > time works out to be not that different. > > > > So, c2hs takes half the time of cpp to do its job? That's not bad at > > all! > > Sadly No. > > The times were including cpp which takes something like 0.1 sec to > process the gtk headers and write them into the .i file. > > It takes 2 sec or so to parse the .i file (and rather longer to > serialise it into our precomp header file for reasons that we still do > not properly understand) Ah! So this is with your serialisation. I forgot about that. > So I hope that you might still accept a few performance patches here and > there. I was thinking of borrowing some techniques from the ghc > lexer/parser which is very fast. It does things like lexing directly > from a memory mapped file rather than going via a lazy string. That also > allows them to use packed strings with zero copy (ie they leave the > actual string data in the memory mapped file!) > > Some of the c2hs data structures could be slightly more compact and/or > stricter in places. Sure, I am more than happy about any performance improvements, and lexing from a memory mapped file sounds like a good plan. Manuel |
From: Duncan C. <dun...@wo...> - 2005-08-22 09:46:38
|
On Sun, 2005-08-21 at 22:44 +1000, Manuel M T Chakravarty wrote: > Duncan Coutts: > > > Not including any .h in foreign imports is not a portable way of using > > > the FFI. > > > > Sorry, I don't think I was clear enough. [snip] > Well, the problem is that -#include is GHC specific. Hmm. So the only portable way is to use the following style? foreign import ccall "gtk/gtk.h gtk_foo" That's unforunate since this form is the least well supported by ghc. Like I said before using this form prevents ghc from doing any cross-module inlining of the C functions. I think we were "cheating" before - at least from c2hs's point of view. Since we were using pre-compiled headers we were not specifying the header file when processing the .chs modules. We only specify it when generating the header. This had the side effect that the ffi decls looked like: foreign import ccall " gtk_foo" > I understand that you want to get rid of the many .h files. My proposal > is to do that by generating > > foreign import ccall safe "gtk/gtk.h gtk_button_pressed" > gtk_button_pressed :: ((Ptr Button) -> (IO ())) > > This gets rid of the .h file, but is still portable. I guess that's the best we can do. It's a shame that there is no other portable way to specify the .h file to the compiler. On the other hand, not allowing the C calls to be inlined across module boundaries into the user's code has certain advantages too. For example it means that user's code does not require the Gtk+ header files to be installed which could be an advantage on Windows. > > We currently do not put the #include "lib.h" into the .chs module since > > we already have to pre-process the .chs module with cpp. That's why we > > specify it on the c2hs and ghc command line. > > Why do you do that? Well many of our modules are not just .chs files but .chs.pp files that contain cpp directives (eg to build things conditional on the gtk version). So we run cpp over the .chs.pp files before running c2hs on them. So if we put the #include "lib.h" into the .chs.pp file then when we run cpp over them it would include the whole of the .h file in place which would not be the right thing. We are currently avoiding using c2hs's own cpp features since they are incompatible with our precompiled header feature. > (The solution that I propose above will still work when you specify > the .h on the c2hs command line.) Yes. Duncan |
From: Manuel M T C. <ch...@cs...> - 2005-08-23 14:00:14
|
Duncan Coutts: > On Sun, 2005-08-21 at 22:44 +1000, Manuel M T Chakravarty wrote: > > Duncan Coutts: > > > > Not including any .h in foreign imports is not a portable way of using > > > > the FFI. > > > > > > Sorry, I don't think I was clear enough. > > [snip] > > > Well, the problem is that -#include is GHC specific. > > Hmm. So the only portable way is to use the following style? > > foreign import ccall "gtk/gtk.h gtk_foo" Yes. > That's unforunate since this form is the least well supported by ghc. > Like I said before using this form prevents ghc from doing any > cross-module inlining of the C functions. It should be alright to pass the -#include options to GHC *in addition* to the specification of the .h file in the import declaration. That's a reasonable approach, as the GHC-specific option is, then, merely a workaround for a GHC-specific problem. > > I understand that you want to get rid of the many .h files. My proposal > > is to do that by generating > > > > foreign import ccall safe "gtk/gtk.h gtk_button_pressed" > > gtk_button_pressed :: ((Ptr Button) -> (IO ())) > > > > This gets rid of the .h file, but is still portable. > > I guess that's the best we can do. It's a shame that there is no other > portable way to specify the .h file to the compiler. > > On the other hand, not allowing the C calls to be inlined across module > boundaries into the user's code has certain advantages too. For example > it means that user's code does not require the Gtk+ header files to be > installed which could be an advantage on Windows. I am also not convinced that you gain much by inlining the FFI calls. A FFI call is pretty expensive compared to an in-Haskell call. > > > We currently do not put the #include "lib.h" into the .chs module since > > > we already have to pre-process the .chs module with cpp. That's why we > > > specify it on the c2hs and ghc command line. > > > > Why do you do that? > > Well many of our modules are not just .chs files but .chs.pp files that > contain cpp directives (eg to build things conditional on the gtk > version). So we run cpp over the .chs.pp files before running c2hs on > them. > > So if we put the #include "lib.h" into the .chs.pp file then when we run > cpp over them it would include the whole of the .h file in place which > would not be the right thing. > > We are currently avoiding using c2hs's own cpp features since they are > incompatible with our precompiled header feature. I realise that this is entirely a decision of the Gtk2Hs developers, but IMHO the Right Thing to do is to drop the pre-compiled header (given that you say that, overall, it doesn't speed the process up much) and to use c2hs' cpp features for conditional compilation. > > (The solution that I propose above will still work when you specify > > the .h on the c2hs command line.) > > Yes. So, I should definitely add that option to c2hs. Manuel |
From: Duncan C. <dun...@wo...> - 2005-08-23 14:21:13
|
On Wed, 2005-08-24 at 00:01 +1000, Manuel M T Chakravarty wrote: > > That's unforunate since this form is the least well supported by ghc. > > Like I said before using this form prevents ghc from doing any > > cross-module inlining of the C functions. > > It should be alright to pass the -#include options to GHC *in addition* > to the specification of the .h file in the import declaration. That's a > reasonable approach, as the GHC-specific option is, then, merely a > workaround for a GHC-specific problem. If it's specified in the ffi decleration then it doesn't need to be specified on the command line. In ghc it's an alternative rather than something needed in addition. > I am also not convinced that you gain much by inlining the FFI calls. A > FFI call is pretty expensive compared to an in-Haskell call. An unsafe call is literally just a C call, without any setup or save/restore. So it's just the cost of push args+call. And ghc can inline them into other modules. Gtk+ has rather a number of very short C functions that do nothing other than just get/set a member in a C struct. However it's probably not a big cost overall. > I realise that this is entirely a decision of the Gtk2Hs developers, but > IMHO the Right Thing to do is to drop the pre-compiled header (given > that you say that, overall, it doesn't speed the process up much) and to > use c2hs' cpp features for conditional compilation. Sure. If we were not using the pre-compiled header thing then we probably would. So if we get back to using the mainline c2hs then we may well use c2hs's cpp features. > > > (The solution that I propose above will still work when you specify > > > the .h on the c2hs command line.) > > > > Yes. > > So, I should definitely add that option to c2hs. I think that would be good. Thanks. I'd be very interested to hear your opinion on the ForeignPtr solution proposed by Axel and my suggestion for preserving backwards compatability. Duncan |
From: Manuel M T C. <ch...@cs...> - 2005-08-23 14:17:35
|
Axel Simon: > Ok, I squeezed your answers together. > > On Sun, 2005-08-21 at 23:22 +1000, Manuel M T Chakravarty wrote: > > Axel Simon: > > > On Thu, 2005-08-18 at 16:39 +0100, Duncan Coutts wrote: > > > > On Thu, 2005-08-18 at 15:45 +0100, Axel Simon wrote: > > > > > > > > > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > > > > > > > > > which is essentially the same as the with* function, but used > > > > > > automatically and inlined at the call site. > > > > > > > > > > It also doesn't pollute the name space. This is more severe that it may > > > > > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > > > > > about the c2hs type declarations in the module. If we were to switch to > > > > > with* functions, we would also have to add these with* functions to the > > > > > export list, depending on weather we use the type somewhere else or not. > > > > > Furthermore we should then hide all these with* functions in Gtk.hs. > > > > > This imposes quite a significant maintenance problem with no apparent > > > > > benefit. > > On Sun, 2005-08-21 at 22:58 +1000, Manuel M T Chakravarty wrote: > > Yes, that's a good point, but what do you do when a ForeignPtr is > > returned as a result? What finalizer do you attach? > > I distinguish return values from arguments. Return values are always > non-wrapped, pure pointers or basic data types. We stick on the > function to free the ForeignPtr and the newtype constructor afterwards. > Automating that is much more involved. To be honest, I am not really keen on an asymmetric approach. This is why we dropped native marshalling of ForeignPtrs in the FFI spec (and that wasn't only my decision, but shared by everybody who participated in the discussion). > [here I admitted that exporting the newtype constructors is no > different than exporting the with* functions.] So, maybe its better to use explicit with functions for call hooks after all. In fun hooks, it's a different story. Having the with functions as default marshallers saves code and there is a standard mechanism to define function specific handling of both argument and result marshallers. > > > b) The inlined version is more flexible when it comes to extensions. We > > > could for example have a tag like "{#call blah bare(PangoFont)#}" which > > > would then let me pass a "Ptr PangoFont" to the function, instead of > > > forcing me to create a ForeignPtr which I'm throwing away immediately > > > afterwards. I just came across this situation. Even nicer, we could add > > > a tag "{#call blah maybe(PangoFont)#}" which takes a "Maybe PangoFont". > > > c2hs could then insert a case distinction to pass a nullPtr to the > > > function if the argument is Nothing. Note that we can't do this at the > > > moment and we have to use the ugly and inefficient (mkPangoFont > > > nullForeignPtr) to pass a Maybe value. In fact, the only reason we have > > > nullForeignPtr is that we can't tell c2hs that we occasionally want to > > > pass a Ptr instead of a ForeignPtr. If we'd stick to the with* approach, > > > we would loose the ability to override the default marshaling for a > > > specific type. > > > > I understand why you like this, but what I am concerned about is that > > you now start to replicate the functionality of fun hooks (which I guess > > were added after you had already done all the design for Gtk2Hs). > > I think they were indeed added after I forked c2hs off. I've just looked > at the documentation of the fun hook. I admit that having a maybe(..) > option is like marshaling magic that looks like it should be in the fun > hook. However a nullPtr can only be passed by c2hs; we always need to > pass a newtype-wrapped nullForeignPtr to the function. Hence it seems > that the nullPtr issue arises as soon as you allow clever things in the > {#pointer #} hooks. > > Similarly, the issue of sometimes passing a PangoFont as (Ptr PangoFont) > instead of the newtype-wrapped ForeignPtr seems to be the result of a > more expressive {#pointer #} hook. We need exceptional cases e.g. when > we bind a function to free a PangoFont (in pre ghc-6.00) which naturally > takes a bare pointer. Yes, but the idea of the call hooks was really that they just automate the call and none of the argument and result marshalling. In contrast, fun hooks do argument and result marshalling (and internally use call hooks to do the actual FFI call). I fully understand that when you made your design, c2hs didn't support all this, so you had to find a different solution, but it seems a bit awkward to add some fun hook functionality to call hooks in the current c2hs. I don't know what your code generators exactly automate, but would it be feasible to generate fun hooks? (I'd be happy to add more intelligence to the default fun hook marshalling if you need that.) > > > I appreciate your attempt to mediate, but the more I think about the > > > Gtk2Hs approach vs. the with* approach, the more I like the former. It > > > shouldn't be too hard to fix it for type synonyms. I could try it this > > > week end, but it really only makes sense if Manuel is happy with it > > > (since Gtk2Hs doesn't use plain type synonyms). > > > > Sorry that my reply was too late. I understand why you like you > > approach, and I sympathise with that, but one problem with changing the > > default behaviour of c2hs is that it would break everybody else's code. > > So, at best, we could make the alternate behaviour an option. > > So what breaks are ForeignPtr hooks, I assume. So when I say > > {#pointer *PangoFont foreign newtype#} > pangoFontGetSize :: PangoFont -> IO Int > pangoFontGetSize pf = > liftM fromIntegral $ {#call unsafe pango_font_get_size#} pf > > our version of c2hs generates: > > newtype PangoFont = PangoFont (ForeignPtr PangoFont) > pangoFontGetSize pf = liftM fromIntegral $ > (\(PangoFont arg1) -> withForeginPtr arg1 $ \arg1 -> > pango_font_get_size arg1) pf > > ... pango_font_get_size :: Ptr PangoFont -> IO CInt ... > > your version generates: > > newtype PangoFont = PangoFont (ForeignPtr PangoFont) > withPangoFont (PangoFront for) = withForeignPtr for > > pangoFontGetSize pf = liftM fromIntegral $ > pango_font_get_size pf > > and in your version I was meant to use withPangoFont pf $ \pfPtr -> ? > If I got this right, then I see how code can be broken. But I think the > abstraction that c2hs gives is not quite right in both cases. In your > version, you let the user declare special treatment for PangoFont but > don't actually honor it. In our case we don't honour return values since > they are always (Ptr PangoFont), just like your arguments (and return > values). The idea behind call hooks is to not do anything about marshalling. It's just the plain function that's automated. All c2hs does is provide the necessary marshalling functionality (in form of the with function), but it's up to the user to do the actual argument marshalling. I think that's a consistent view. > One could fix the pointer hook by specifying the finalizer and let c2hs > automatically create the corresponding code. I don't really like that, > though, since sometimes things are too complicated to be left to c2hs. I agree. > > BTW, I just had a look at c2hs' source and unfortunately our two source > > trees have diverged here. I guess when I added fun hooks, I changed the > > type of PointerMap already, so a patch from your tree wouldn't apply > > cleanly. > > Can you recover what has changed? Your darcs repository does not reveal > any information on c2hs/chs/CHS.hs. Did you get the repository with --partial? darcs changes c2hs/chs/CHS.hs gives me the history back to April 2001. (I converted the CVS to darcs with taylor.pl.) Cheers, Manuel |
From: Axel S. <A....@ke...> - 2005-08-23 16:10:31
|
Sorry, my last email took a bit longer to write, so I missed your reply. On Wed, 2005-08-24 at 00:18 +1000, Manuel M T Chakravarty wrote: > > > I understand why you like this, but what I am concerned about is that > > > you now start to replicate the functionality of fun hooks (which I guess > > > were added after you had already done all the design for Gtk2Hs). > > > > I think they were indeed added after I forked c2hs off. I've just looked > > at the documentation of the fun hook. I admit that having a maybe(..) > > option is like marshaling magic that looks like it should be in the fun > > hook. However a nullPtr can only be passed by c2hs; we always need to > > pass a newtype-wrapped nullForeignPtr to the function. Hence it seems > > that the nullPtr issue arises as soon as you allow clever things in the > > {#pointer #} hooks. > > > > Similarly, the issue of sometimes passing a PangoFont as (Ptr PangoFont) > > instead of the newtype-wrapped ForeignPtr seems to be the result of a > > more expressive {#pointer #} hook. We need exceptional cases e.g. when > > we bind a function to free a PangoFont (in pre ghc-6.00) which naturally > > takes a bare pointer. > > Yes, but the idea of the call hooks was really that they just automate > the call and none of the argument and result marshalling. In contrast, > fun hooks do argument and result marshalling (and internally use call > hooks to do the actual FFI call). I see your point of view. However, I really, really don't want to keep the call-hooks as they are: a) I don't think that wrapping and unwrapping of newtype and ForeignPtrs is marshaling in the sense that data is changed/copied. Hence from the design point of view, it is inconsistent that call hooks don't take their pointer arguments as the user has declared them. For consistency, call hooks should already wrap and unwrap pointers (but only pointers). The extension with maybe(..) and potentially bare(..) might be necessary, which I admit is a pain. b) I absolutely dread the thought of changing all of gtk2hs. No matter how good an automatic code generator and merging tools are, it is going to be much more work than 2-3 hours to incorporate your call-hook style to Gtk2Hs. I'm sorry that I'm kind of saying, "my way or the high way", but it would be a lot of work for rather little benefit. I think any solution that is acceptable to me (it seems I have to exclude Duncan here) will have to make do with changing your c2hs function hooks to Gtk2hs' c2hs semantics at least for *the arguments*. > I fully understand that when you made your design, c2hs didn't support > all this, so you had to find a different solution, but it seems a bit > awkward to add some fun hook functionality to call hooks in the current > c2hs. > > I don't know what your code generators exactly automate, but would it be > feasible to generate fun hooks? (I'd be happy to add more intelligence > to the default fun hook marshalling if you need that.) The code generator spits out a complete module for a widget. You then have to modify all functions that take arrays, or return GSList or does anything besides setting and getting a value. If we change things on large scale it would mean that we replace the trivial function with the stuff the code generator produces and hand edit all non-trivial functions. > > If I got this right, then I see how code can be broken. But I think the > > abstraction that c2hs gives is not quite right in both cases. In your > > version, you let the user declare special treatment for PangoFont but > > don't actually honor it. In our case we don't honour return values since > > they are always (Ptr PangoFont), just like your arguments (and return > > values). > > The idea behind call hooks is to not do anything about marshalling. > It's just the plain function that's automated. All c2hs does is provide > the necessary marshalling functionality (in form of the with function), > but it's up to the user to do the actual argument marshalling. I think > that's a consistent view. I guess it is consistent to provide the bare minimum and a fully automated way. I still think it is slightly inconsistent that the user declares a pointer P but then only gets Ptr P in all the call arguments and returns. Maybe we can have something in between: A call hook that marshals pointers as described in "The Right Thing" table, but otherwise behaves like {#call#}. > > One could fix the pointer hook by specifying the finalizer and let c2hs > > automatically create the corresponding code. I don't really like that, > > though, since sometimes things are too complicated to be left to c2hs. > > I agree. I think the "Right Thing" is doable. (I think it's time to rename that approach). > > > BTW, I just had a look at c2hs' source and unfortunately our two source > > > trees have diverged here. I guess when I added fun hooks, I changed the > > > type of PointerMap already, so a patch from your tree wouldn't apply > > > cleanly. > > > > Can you recover what has changed? Your darcs repository does not reveal > > any information on c2hs/chs/CHS.hs. > > Did you get the repository with --partial? > > darcs changes c2hs/chs/CHS.hs > Oh, no, I just looked at the web interface. Axel. |
From: Axel S. <A....@ke...> - 2005-08-24 07:13:27
|
Good morning, I had an idea yesterday: Maybe we should really have a new hook or a new keyword. We can keep the {#fun#} and {#call#} hooks as in Manuel's c2hs and add a keyword "auto" such that {#call auto blah#} {#fun auto blah#} {#get auto blah#} {#set auto blah#} corresponds to automatic unwrapping/unForeignPtr'ing as described in the "Right Thing" proposal. This would be a useful addition to c2hs users and a simple search-and-replace on all our files for us. We would still need to fix all return values that are pointers, but that is a worthwhile change. Furthermore, by retaining the normal {#call #} hook, we can still pass nullPtrs instead of nullForeignPtrs (and use with with* functions to pack/unpack the pointer). Are there any disadvantages? Axel |
From: Duncan C. <dun...@wo...> - 2005-08-24 08:58:29
|
On Wed, 2005-08-24 at 08:12 +0100, Axel Simon wrote: > Good morning, > > I had an idea yesterday: Maybe we should really have a new hook or a new > keyword. We can keep the {#fun#} and {#call#} hooks as in Manuel's c2hs > and add a keyword "auto" such that > > {#call auto blah#} > {#fun auto blah#} > {#get auto blah#} > {#set auto blah#} > > corresponds to automatic unwrapping/unForeignPtr'ing as described in the > "Right Thing" proposal. This would be a useful addition to c2hs users > and a simple search-and-replace on all our files for us. We would still > need to fix all return values that are pointers, but that is a > worthwhile change. Furthermore, by retaining the normal {#call #} hook, > we can still pass nullPtrs instead of nullForeignPtrs (and use with > with* functions to pack/unpack the pointer). > > Are there any disadvantages? Sounds ok to me. Certainly less work for us! :-) I can have the code generator generate ordinary {# call #} hooks when any of the ForeignPtr args/result can be NULL. There are about 120 of these functions that the code generator already knows about. Duncan |
From: Manuel M T C. <ch...@cs...> - 2005-08-25 14:00:08
|
I didn't have the time to look into this today (and go through all the other messages in detail), but this might be a good compromise. Manuel Axel Simon: > I had an idea yesterday: Maybe we should really have a new hook or a new > keyword. We can keep the {#fun#} and {#call#} hooks as in Manuel's c2hs > and add a keyword "auto" such that > > {#call auto blah#} > {#fun auto blah#} > {#get auto blah#} > {#set auto blah#} > > corresponds to automatic unwrapping/unForeignPtr'ing as described in the > "Right Thing" proposal. This would be a useful addition to c2hs users > and a simple search-and-replace on all our files for us. We would still > need to fix all return values that are pointers, but that is a > worthwhile change. Furthermore, by retaining the normal {#call #} hook, > we can still pass nullPtrs instead of nullForeignPtrs (and use with > with* functions to pack/unpack the pointer). > > Are there any disadvantages? > > Axel > |
From: Axel S. <A....@ke...> - 2005-08-18 23:17:15
|
On Thu, 2005-08-18 at 16:39 +0100, Duncan Coutts wrote: > On Thu, 2005-08-18 at 15:45 +0100, Axel Simon wrote: > > > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > > > which is essentially the same as the with* function, but used > > > automatically and inlined at the call site. > > > > It also doesn't pollute the name space. This is more severe that it may > > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > > about the c2hs type declarations in the module. If we were to switch to > > with* functions, we would also have to add these with* functions to the > > export list, depending on weather we use the type somewhere else or not. > > Furthermore we should then hide all these with* functions in Gtk.hs. > > This imposes quite a significant maintenance problem with no apparent > > benefit. > > I'm not sure that it would be much different. With our solution we have > to export the newtype constructor name to other binding modules and make > sure that they do not escape from the top level Gtk module. True, good point. > It would be nice if c2hs used the with* functions automatically like we > have in our c2hs fork (since for one thing it makes it easier for people > to experiment and switch between foriegn/newtype/foriegn+newtype/raw), > but we could change our code to use the with* functions reasonably > easily by changing our code generator. The last option means we have to rewrite every single function binding. I'm kind of against that. I still think that the Gtk2Hs version of c2hs is the right way to proceed: a) The .chi files were introduced since c2hs needed to know how a specific type needs to be marshaled. Conveying this information mostly though exporting Haskell functions is just circumventing the .chi file, where the information belongs. This reason is rather philosophical. b) The inlined version is more flexible when it comes to extensions. We could for example have a tag like "{#call blah bare(PangoFont)#}" which would then let me pass a "Ptr PangoFont" to the function, instead of forcing me to create a ForeignPtr which I'm throwing away immediately afterwards. I just came across this situation. Even nicer, we could add a tag "{#call blah maybe(PangoFont)#}" which takes a "Maybe PangoFont". c2hs could then insert a case distinction to pass a nullPtr to the function if the argument is Nothing. Note that we can't do this at the moment and we have to use the ugly and inefficient (mkPangoFont nullForeignPtr) to pass a Maybe value. In fact, the only reason we have nullForeignPtr is that we can't tell c2hs that we occasionally want to pass a Ptr instead of a ForeignPtr. If we'd stick to the with* approach, we would loose the ability to override the default marshaling for a specific type. I appreciate your attempt to mediate, but the more I think about the Gtk2Hs approach vs. the with* approach, the more I like the former. It shouldn't be too hard to fix it for type synonyms. I could try it this week end, but it really only makes sense if Manuel is happy with it (since Gtk2Hs doesn't use plain type synonyms). Axel. |
From: Manuel M T C. <ch...@cs...> - 2005-08-21 13:21:11
|
Axel Simon: > On Thu, 2005-08-18 at 16:39 +0100, Duncan Coutts wrote: > > On Thu, 2005-08-18 at 15:45 +0100, Axel Simon wrote: > > > > > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > > > > > which is essentially the same as the with* function, but used > > > > automatically and inlined at the call site. > > > > > > It also doesn't pollute the name space. This is more severe that it may > > > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > > > about the c2hs type declarations in the module. If we were to switch to > > > with* functions, we would also have to add these with* functions to the > > > export list, depending on weather we use the type somewhere else or not. > > > Furthermore we should then hide all these with* functions in Gtk.hs. > > > This imposes quite a significant maintenance problem with no apparent > > > benefit. > > > > I'm not sure that it would be much different. With our solution we have > > to export the newtype constructor name to other binding modules and make > > sure that they do not escape from the top level Gtk module. > > True, good point. > > > > It would be nice if c2hs used the with* functions automatically like we > > have in our c2hs fork (since for one thing it makes it easier for people > > to experiment and switch between foriegn/newtype/foriegn+newtype/raw), > > but we could change our code to use the with* functions reasonably > > easily by changing our code generator. > > The last option means we have to rewrite every single function binding. > I'm kind of against that. > > I still think that the Gtk2Hs version of c2hs is the right way to > proceed: > > a) The .chi files were introduced since c2hs needed to know how a > specific type needs to be marshaled. Conveying this information mostly > though exporting Haskell functions is just circumventing the .chi file, > where the information belongs. This reason is rather philosophical. Fair enough, but I need to know how you handle ForeignPtrs as results. > b) The inlined version is more flexible when it comes to extensions. We > could for example have a tag like "{#call blah bare(PangoFont)#}" which > would then let me pass a "Ptr PangoFont" to the function, instead of > forcing me to create a ForeignPtr which I'm throwing away immediately > afterwards. I just came across this situation. Even nicer, we could add > a tag "{#call blah maybe(PangoFont)#}" which takes a "Maybe PangoFont". > c2hs could then insert a case distinction to pass a nullPtr to the > function if the argument is Nothing. Note that we can't do this at the > moment and we have to use the ugly and inefficient (mkPangoFont > nullForeignPtr) to pass a Maybe value. In fact, the only reason we have > nullForeignPtr is that we can't tell c2hs that we occasionally want to > pass a Ptr instead of a ForeignPtr. If we'd stick to the with* approach, > we would loose the ability to override the default marshaling for a > specific type. I understand why you like this, but what I am concerned about is that you now start to replicate the functionality of fun hooks (which I guess were added after you had already done all the design for Gtk2Hs). > I appreciate your attempt to mediate, but the more I think about the > Gtk2Hs approach vs. the with* approach, the more I like the former. It > shouldn't be too hard to fix it for type synonyms. I could try it this > week end, but it really only makes sense if Manuel is happy with it > (since Gtk2Hs doesn't use plain type synonyms). Sorry that my reply was too late. I understand why you like you approach, and I sympathise with that, but one problem with changing the default behaviour of c2hs is that it would break everybody else's code. So, at best, we could make the alternate behaviour an option. BTW, I just had a look at c2hs' source and unfortunately our two source trees have diverged here. I guess when I added fun hooks, I changed the type of PointerMap already, so a patch from your tree wouldn't apply cleanly. That makes things more complicated, but I'd still like to solve this somehow. Manuel |
From: Axel S. <A....@ke...> - 2005-08-21 14:27:23
|
Ok, I squeezed your answers together. On Sun, 2005-08-21 at 23:22 +1000, Manuel M T Chakravarty wrote: > Axel Simon: > > On Thu, 2005-08-18 at 16:39 +0100, Duncan Coutts wrote: > > > On Thu, 2005-08-18 at 15:45 +0100, Axel Simon wrote: > > > > > > > On Thu, 2005-08-18 at 14:55 +0100, Duncan Coutts wrote: > > > > > > > > which is essentially the same as the with* function, but used > > > > > automatically and inlined at the call site. > > > > > > > > It also doesn't pollute the name space. This is more severe that it may > > > > seem at first. In Gtk2Hs we often {#import #} modules, so that c2hs know > > > > about the c2hs type declarations in the module. If we were to switch to > > > > with* functions, we would also have to add these with* functions to the > > > > export list, depending on weather we use the type somewhere else or not. > > > > Furthermore we should then hide all these with* functions in Gtk.hs. > > > > This imposes quite a significant maintenance problem with no apparent > > > > benefit. On Sun, 2005-08-21 at 22:58 +1000, Manuel M T Chakravarty wrote: > Yes, that's a good point, but what do you do when a ForeignPtr is > returned as a result? What finalizer do you attach? I distinguish return values from arguments. Return values are always non-wrapped, pure pointers or basic data types. We stick on the function to free the ForeignPtr and the newtype constructor afterwards. Automating that is much more involved. [here I admitted that exporting the newtype constructors is no different than exporting the with* functions.] > > I still think that the Gtk2Hs version of c2hs is the right way to > > proceed: > > > > a) The .chi files were introduced since c2hs needed to know how a > > specific type needs to be marshaled. Conveying this information mostly > > though exporting Haskell functions is just circumventing the .chi file, > > where the information belongs. This reason is rather philosophical. > > Fair enough, but I need to know how you handle ForeignPtrs as results. See above. > > b) The inlined version is more flexible when it comes to extensions. We > > could for example have a tag like "{#call blah bare(PangoFont)#}" which > > would then let me pass a "Ptr PangoFont" to the function, instead of > > forcing me to create a ForeignPtr which I'm throwing away immediately > > afterwards. I just came across this situation. Even nicer, we could add > > a tag "{#call blah maybe(PangoFont)#}" which takes a "Maybe PangoFont". > > c2hs could then insert a case distinction to pass a nullPtr to the > > function if the argument is Nothing. Note that we can't do this at the > > moment and we have to use the ugly and inefficient (mkPangoFont > > nullForeignPtr) to pass a Maybe value. In fact, the only reason we have > > nullForeignPtr is that we can't tell c2hs that we occasionally want to > > pass a Ptr instead of a ForeignPtr. If we'd stick to the with* approach, > > we would loose the ability to override the default marshaling for a > > specific type. > > I understand why you like this, but what I am concerned about is that > you now start to replicate the functionality of fun hooks (which I guess > were added after you had already done all the design for Gtk2Hs). I think they were indeed added after I forked c2hs off. I've just looked at the documentation of the fun hook. I admit that having a maybe(..) option is like marshaling magic that looks like it should be in the fun hook. However a nullPtr can only be passed by c2hs; we always need to pass a newtype-wrapped nullForeignPtr to the function. Hence it seems that the nullPtr issue arises as soon as you allow clever things in the {#pointer #} hooks. Similarly, the issue of sometimes passing a PangoFont as (Ptr PangoFont) instead of the newtype-wrapped ForeignPtr seems to be the result of a more expressive {#pointer #} hook. We need exceptional cases e.g. when we bind a function to free a PangoFont (in pre ghc-6.00) which naturally takes a bare pointer. > > I appreciate your attempt to mediate, but the more I think about the > > Gtk2Hs approach vs. the with* approach, the more I like the former. It > > shouldn't be too hard to fix it for type synonyms. I could try it this > > week end, but it really only makes sense if Manuel is happy with it > > (since Gtk2Hs doesn't use plain type synonyms). > > Sorry that my reply was too late. I understand why you like you > approach, and I sympathise with that, but one problem with changing the > default behaviour of c2hs is that it would break everybody else's code. > So, at best, we could make the alternate behaviour an option. So what breaks are ForeignPtr hooks, I assume. So when I say {#pointer *PangoFont foreign newtype#} pangoFontGetSize :: PangoFont -> IO Int pangoFontGetSize pf = liftM fromIntegral $ {#call unsafe pango_font_get_size#} pf our version of c2hs generates: newtype PangoFont = PangoFont (ForeignPtr PangoFont) pangoFontGetSize pf = liftM fromIntegral $ (\(PangoFont arg1) -> withForeginPtr arg1 $ \arg1 -> pango_font_get_size arg1) pf ... pango_font_get_size :: Ptr PangoFont -> IO CInt ... your version generates: newtype PangoFont = PangoFont (ForeignPtr PangoFont) withPangoFont (PangoFront for) = withForeignPtr for pangoFontGetSize pf = liftM fromIntegral $ pango_font_get_size pf and in your version I was meant to use withPangoFont pf $ \pfPtr -> ? If I got this right, then I see how code can be broken. But I think the abstraction that c2hs gives is not quite right in both cases. In your version, you let the user declare special treatment for PangoFont but don't actually honor it. In our case we don't honour return values since they are always (Ptr PangoFont), just like your arguments (and return values). One could fix the pointer hook by specifying the finalizer and let c2hs automatically create the corresponding code. I don't really like that, though, since sometimes things are too complicated to be left to c2hs. For example, we often do something special if the return value is a null pointer. Letting c2hs wrap the nullPtr automatically in a newtype +ForeignPtr declaration is fatal, since eventually the finalizer will be called on the nullPtr. > BTW, I just had a look at c2hs' source and unfortunately our two source > trees have diverged here. I guess when I added fun hooks, I changed the > type of PointerMap already, so a patch from your tree wouldn't apply > cleanly. Can you recover what has changed? Your darcs repository does not reveal any information on c2hs/chs/CHS.hs. Thanks, Axel. |
From: Duncan C. <dun...@wo...> - 2005-08-23 11:24:24
|
On Sun, 2005-08-21 at 15:26 +0100, Axel Simon wrote: > > Sorry that my reply was too late. I understand why you like you > > approach, and I sympathise with that, but one problem with changing the > > default behaviour of c2hs is that it would break everybody else's code. > > So, at best, we could make the alternate behaviour an option. > > So what breaks are ForeignPtr hooks, I assume. So when I say > > {#pointer *PangoFont foreign newtype#} > pangoFontGetSize :: PangoFont -> IO Int > pangoFontGetSize pf = > liftM fromIntegral $ {#call unsafe pango_font_get_size#} pf > > our version of c2hs generates: > > newtype PangoFont = PangoFont (ForeignPtr PangoFont) > pangoFontGetSize pf = liftM fromIntegral $ > (\(PangoFont arg1) -> withForeginPtr arg1 $ \arg1 -> > pango_font_get_size arg1) pf > > ... pango_font_get_size :: Ptr PangoFont -> IO CInt ... > > your version generates: > > newtype PangoFont = PangoFont (ForeignPtr PangoFont) > withPangoFont (PangoFront for) = withForeignPtr for > > pangoFontGetSize pf = liftM fromIntegral $ > pango_font_get_size pf > > and in your version I was meant to use withPangoFont pf $ \pfPtr -> ? > If I got this right, then I see how code can be broken. But I think the > abstraction that c2hs gives is not quite right in both cases. In your > version, you let the user declare special treatment for PangoFont but > don't actually honor it. In our case we don't honour return values since > they are always (Ptr PangoFont), just like your arguments (and return > values). > > One could fix the pointer hook by specifying the finalizer and let c2hs > automatically create the corresponding code. I don't really like that, > though, since sometimes things are too complicated to be left to c2hs. > For example, we often do something special if the return value is a null > pointer. Letting c2hs wrap the nullPtr automatically in a newtype > +ForeignPtr declaration is fatal, since eventually the finalizer will be > called on the nullPtr. So what is the "right thing" to do here do you think? Unwrapping on the "in" side only seems unsatisfactory, but wrapping up on the "out" side is hard to do in general because of null values etc. If the approaches are equally good we should just bit the bullet and convert our code to use the with* functions. It wouldn't take me more than two or three hours to merge everything after changing the code generator. On the other hand if we can improve things for all users then we should try and think about what the better soultion should be. Although there is always the problem of backwards compatability for existing c2hs users. Duncan |
From: Axel S. <A....@ke...> - 2005-08-23 12:47:30
|
On Tue, 2005-08-23 at 12:25 +0100, Duncan Coutts wrote: > > > > One could fix the pointer hook by specifying the finalizer and let c2hs > > automatically create the corresponding code. I don't really like that, > > though, since sometimes things are too complicated to be left to c2hs. > > For example, we often do something special if the return value is a null > > pointer. Letting c2hs wrap the nullPtr automatically in a newtype > > +ForeignPtr declaration is fatal, since eventually the finalizer will be > > called on the nullPtr. > > So what is the "right thing" to do here do you think? Unwrapping on the > "in" side only seems unsatisfactory, but wrapping up on the "out" side > is hard to do in general because of null values etc. The Right Thing (tm): (a) Make c2hs consistent as to how it deals with type synonyms. A ForeignPtr declaration needs to specify a finalizer: {#pointer PangoFont foreign(gobject_unref) newtype #} Any function that returns a pointer to a PangoFont will then automatically be turned into a ForeignPtr with the mentioned finalizer and wrapped in the newtype. This behaviour breaks backwards compatibility for other users of c2hs who use {#pointer foreign#}. It will break the 207 gobject constructors in Gtk2Hs plus perhaps 20 odd constructors of other structures. (b) Add a new maybe(<type>) flag to call, fun, get and set hooks. This hook would tell c2hs that the pointer can be potentially NULL and it will take and return a Maybe type for arguments and return values of that type. It will break all code that uses functions that can return pointers. This will break quite a few functions in Gtk2Hs, but it will make all these functions shorter. Example: pangoFontDescribe :: (Maybe PangoFont) -> IO (Maybe PangoFontDescription) pangoFontDescribe mPf = {#call unsafe maybe(PangoFont) maybe(PangoFontDescription) pango_font_describe#} mPf pangoItemGetFont :: PangoItem -> IO (Maybe PangoFont) pangoItemGetFont pi = {#get maybe(PangoFont) PangoItem, font#} pi > If the approaches are equally good we should just bit the bullet and > convert our code to use the with* functions. It wouldn't take me more > than two or three hours to merge everything after changing the code > generator. You are proposing to rewrite the whole binding!? I grepped for "::" in the .chs and .chs.pp files. There are around 5300 functions. You code generator would be able to generate say 80% of that. Still merging old and new code still takes probably 20min till an hour per module. Some modules like Pango have to be written manually. If it would be that quick, the binding should be finished by now. > On the other hand if we can improve things for all users then we should > try and think about what the better soultion should be. Although there > is always the problem of backwards compatability for existing c2hs > users. I think I would be happy to do the fixes for the approach "The Right Thing". But I don't want to convert all functions in Gtk2Hs for the sole benefit of being able to use an external c2hs. |
From: Duncan C. <dun...@wo...> - 2005-08-23 13:18:50
|
On Tue, 2005-08-23 at 13:45 +0100, Axel Simon wrote: > On Tue, 2005-08-23 at 12:25 +0100, Duncan Coutts wrote: > > > > > > > One could fix the pointer hook by specifying the finalizer and let c2hs > > > automatically create the corresponding code. I don't really like that, > > > though, since sometimes things are too complicated to be left to c2hs. > > > For example, we often do something special if the return value is a null > > > pointer. Letting c2hs wrap the nullPtr automatically in a newtype > > > +ForeignPtr declaration is fatal, since eventually the finalizer will be > > > called on the nullPtr. > > > > So what is the "right thing" to do here do you think? Unwrapping on the > > "in" side only seems unsatisfactory, but wrapping up on the "out" side > > is hard to do in general because of null values etc. > > The Right Thing (tm): > > (a) Make c2hs consistent as to how it deals with type synonyms. A > ForeignPtr declaration needs to specify a finalizer: > {#pointer PangoFont foreign(gobject_unref) newtype #} Ok. > Any function that returns a pointer to a PangoFont will then > automatically be turned into a ForeignPtr with the mentioned finalizer > and wrapped in the newtype. This behaviour breaks backwards > compatibility for other users of c2hs who use {#pointer foreign#}. It > will break the 207 gobject constructors in Gtk2Hs plus perhaps 20 odd > constructors of other structures. Sounds good. To preserve backwards compatability for other c2hs users perhaps the finalizer could be optional but stronly reccomended. That way, if you specify no finaliser then you get the current behaviour, ie no wrapping of results in a ForeignPtr. This could alos control wether you get the current behaviour of no-unwrapping of the ForeignPtrs too. So if I say: {#pointer *PangoFont foreign newtype#} pangoFontGetSize :: PangoFont -> IO Int pangoFontGetSize pf = liftM fromIntegral $ {#call unsafe pango_font_get_size#} pf then we get the current output: newtype PangoFont = PangoFont (ForeignPtr PangoFont) withPangoFont (PangoFront for) = withForeignPtr for pangoFontGetSize pf = liftM fromIntegral $ pango_font_get_size pf and I have to do the wrapping unwrapping manually but if I say: {#pointer *PangoFont foreign(gobject_unref) newtype#} then we get the full unwrapping/wrapping behaviour: newtype PangoFont = PangoFont (ForeignPtr PangoFont) pangoFontGetSize pf = liftM fromIntegral $ (\(PangoFont arg1) -> withForeginPtr arg1 $ \arg1 -> pango_font_get_size arg1 >>= newForeginPtr gobject_unref) pf Or as Manuel suggest it might be better to but the wrapping/unwrapping code in the delayed code at the end with the ffi import decl. > (b) Add a new maybe(<type>) flag to call, fun, get and set hooks. This > hook would tell c2hs that the pointer can be potentially NULL and it > will take and return a Maybe type for arguments and return values of > that type. It will break all code that uses functions that can return > pointers. This will break quite a few functions in Gtk2Hs, but it will > make all these functions shorter. So this allows you to specify maybe for arguments and return values. I wonder if that's really necessary. Perhaps just allowing it for return values, since that's the case that causes problems for ForeignPtrs and finalisers. Then it'd just be: {#call unsafe maybe gtk_foo_bar #} foo and get back: Maybe Foo And there is a danger in specifying per-type rather than per-parameter that you might have two parameters of the same type but one can me NULL and the other not. It's true that at the moment we have to make a nullForeignPtr by nefarious means since our current c2hs always expects to unwrap a ForeignPtr, even when we really intend to pass NULL. So somethig to support passing NULLs where a ForeignPtr would normally go might be nice. > Example: > > pangoFontDescribe :: (Maybe PangoFont) -> > IO (Maybe PangoFontDescription) > pangoFontDescribe mPf = > {#call unsafe maybe(PangoFont) > maybe(PangoFontDescription) pango_font_describe#} mPf > > pangoItemGetFont :: PangoItem -> IO (Maybe PangoFont) > pangoItemGetFont pi = {#get maybe(PangoFont) PangoItem, font#} pi > > > If the approaches are equally good we should just bit the bullet and > > convert our code to use the with* functions. It wouldn't take me more > > than two or three hours to merge everything after changing the code > > generator. > I think I would be happy to do the fixes for the approach "The Right > Thing". But I don't want to convert all functions in Gtk2Hs for the sole > benefit of being able to use an external c2hs. I think it could be done in a backwards compatabile way for other c2hs users. And for us we wouldn't have to modify most functions if we move to specifying the finaliser in the {# pointer #} hook, just those ones that return an object. There are far fewer of these and they can be found with a simple text search. It's all the makeNew(G)Object uses. So if this same fearure were added to both c2hs branches, then it would provide a common upgrade path. Duncan |