From: William S F. <ws...@fu...> - 2014-06-06 06:51:36
|
Iwill be focusing next on the Scilab integration into master. Simon, can you please merge master into the gsoc2012-scilab branch ? Since your last merge from master in March, there have unfortunately (for you) been a number of build changeswhich you will need to review and make sure is up to date on your branchbefore we can merge to master. - Please look at all the makefile changes and replicate what has happened on master into your files. The changes are aimed at providing out of source builds and a better clean. - Please review your .gitignore changes and makesure the master version does not have equivalentsto your changes. - Note that the latest Travis testing might break as it now does out of source builds and checks that make maintainer-clean is working (ie no unexpected files leftafter a clean). I'll be posting further review comments to this email thread. William |
From: Simon M. <sim...@sc...> - 2014-06-06 13:44:27
|
With pleasure :-) I have begun to merge. I don't think the "out of sources build" changes will be hard to integrate, but we'll see. Simon Le 06/06/2014 08:51, William S Fulton a écrit : > Iwill be focusing next on the Scilab integration into master. > > Simon, can you please merge master into the gsoc2012-scilab branch ? > Since your last merge from master in March, there have unfortunately > (for you) been a number of build changeswhich you will need to review > and make sure is up to date on your branchbefore we can merge to master. > > - Please look at all the makefile changes and replicate what has > happened on master into your files. The changes are aimed at providing > out of source builds and a better clean. > - Please review your .gitignore changes and makesure the master > version does not have equivalentsto your changes. > - Note that the latest Travis testing might break as it now does out > of source builds and checks that make maintainer-clean is working (ie > no unexpected files leftafter a clean). > > I'll be posting further review comments to this email thread. > > William > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/NeoTech > > > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel |
From: Simon M. <sim...@sc...> - 2014-06-10 15:54:38
|
William, I had indeed some troubles with the out of source build but it works now. I have a broken test to fix, and a couple of things to check and do, but it won't take long. When do you want to make the release ? Simon Le 06/06/2014 15:44, Simon Marchetto a écrit : > With pleasure :-) I have begun to merge. > > I don't think the "out of sources build" changes will be hard to > integrate, but we'll see. > > Simon > > Le 06/06/2014 08:51, William S Fulton a écrit : >> Iwill be focusing next on the Scilab integration into master. >> >> Simon, can you please merge master into the gsoc2012-scilab branch ? >> Since your last merge from master in March, there have unfortunately >> (for you) been a number of build changeswhich you will need to review >> and make sure is up to date on your branchbefore we can merge to master. >> >> - Please look at all the makefile changes and replicate what has >> happened on master into your files. The changes are aimed at >> providing out of source builds and a better clean. >> - Please review your .gitignore changes and makesure the master >> version does not have equivalentsto your changes. >> - Note that the latest Travis testing might break as it now does out >> of source builds and checks that make maintainer-clean is working (ie >> no unexpected files leftafter a clean). >> >> I'll be posting further review comments to this email thread. >> >> William >> >> >> ------------------------------------------------------------------------------ >> Learn Graph Databases - Download FREE O'Reilly Book >> "Graph Databases" is the definitive new guide to graph databases and their >> applications. Written by three acclaimed leaders in the field, >> this first edition is now available. Download your free book today! >> http://p.sf.net/sfu/NeoTech >> >> >> _______________________________________________ >> Swig-devel mailing list >> Swi...@li... >> https://lists.sourceforge.net/lists/listinfo/swig-devel > > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/NeoTech > > > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel |
From: William S F. <ws...@fu...> - 2014-06-12 06:35:30
|
On 10/06/14 16:54, Simon Marchetto wrote: > When do you want to make the release ? > As soon as the Scilab module is merged into master. Hopefully in a couple of weeks time, depending on what is found during review and if you have the time to address issues as they are found. I've read the Scilab.html chapter and have a few initial comments: Building the module section: - There is no ./scilab-cli script - Can you describe more what the point of libexample.c is? Why is it needed in addition to the _wrap.c file? Why is the file always called loader.sce ? In the other languages, you get a file named after the module name, so example_loader.sce would be more appropriate. Likewise for builder.sce. The output of 'swig -scilab -help' does not match the documented command line options. Can you also format this output to line up like the other options? Can you also make the option names a bit more readable, I suggest changing: -addcflag => -addcflags -addldflag => -addldflags -addsrc => -addsources -vbl <level> => -buildverbosity <level> -intmod => -internalmodule -ol <library name> => -outputlibrary <name> "Note: in fact, it is not necessary to include the library typemaps.i, this one is included by default. " This is inconsistent with the other language, please change it to not include by default. Unsigned integers are handled quite differently to signed integers wrt conversion to double. I don't see any logic in this, I think they should be handled the same, that is, unsigned and signed converted to and from doubles equally. Can you elaborate why this shouldn't be done? Named typemaps use upper case as parameter names in SWIG. Can you modify the typemaps included by matrix.i to use upper case (and correct the html docs too). You could then drop the 'matrix' prefix, eg: matrixIn => IN In the docs: "Scilab is supported from version 5.3.3". but configure.ac checks for 5.3 or higher. In configure: checking for scilab... scilab configure: Scilab version: 5.2.0.1266391513 checking for Scilab version is 5.3 or higher... no checking for Scilab startup options... -nwni -nb checking for Scilab header files... /usr/include there is no need to do the remaining checks if the version number is not high enough. William |
From: Simon M. <sim...@sc...> - 2014-06-12 10:22:34
|
Le 12/06/2014 08:35, William S Fulton a écrit : > On 10/06/14 16:54, Simon Marchetto wrote: >> When do you want to make the release ? >> > As soon as the Scilab module is merged into master. Hopefully in a > couple of weeks time, depending on what is found during review and if > you have the time to address issues as they are found. > > > I've read the Scilab.html chapter and have a few initial comments: > > Building the module section: > - There is no ./scilab-cli script Done => replaced by "scilab" > - Can you describe more what the point of libexample.c is? Why is it > needed in addition to the _wrap.c file? > Done => added the following short explanation "used by Scilab at run time to link each module new declared function in Scilab to the related wrapped C/C++function (ex: foo() in Scilab is routed to the C/C++ function wrap_foo())" I will add at the end of the doc some other references to Scilab documentation (describing Scilab external modules). You may find some details there. > Why is the file always called loader.sce ? In the other languages, you > get a file named after the module name, so example_loader.sce would be > more appropriate. Likewise for builder.sce. > Historical reason. I agree with you. I have to see if we can change it in the future version of Scilab. > The output of 'swig -scilab -help' does not match the documented > command line options. Can you also format this output to line up like > the other options? > > Can you also make the option names a bit more readable, I suggest > changing: > > -addcflag => -addcflags > -addldflag => -addldflags > -addsrc => -addsources > -vbl <level> => -buildverbosity <level> > -intmod => -internalmodule > -ol <library name> => -outputlibrary <name> > > "Note: in fact, it is not necessary to include the library typemaps.i, > this one is included by default. " This is inconsistent with the other > language, please change it to not include by default. > => I can run the example I gave in the documentation in Scilab and in Python, without including typemaps.i. Is it because this example does not need typemaps.i ? Or because typemaps.i is included by default ? In any case, I did not intend to include it by default. I did a search now, I couldn't find any such include in Lib/scilab, but maybe I missed it. > Unsigned integers are handled quite differently to signed integers wrt > conversion to double. I don't see any logic in this, I think they > should be handled the same, that is, unsigned and signed converted to > and from doubles equally. Can you elaborate why this shouldn't be done? > OK I will fix it. > Named typemaps use upper case as parameter names in SWIG. > Can you modify the typemaps included by matrix.i to use upper case > (and correct the html docs too). You could then drop the 'matrix' > prefix, eg: > matrixIn => IN > OK > In the docs: "Scilab is supported from version 5.3.3". but > configure.ac checks for 5.3 or higher. > > In configure: > > checking for scilab... scilab > configure: Scilab version: 5.2.0.1266391513 > checking for Scilab version is 5.3 or higher... no > checking for Scilab startup options... -nwni -nb > checking for Scilab header files... /usr/include > Done => configure checks 5.3.3 instead of 5.3. > there is no need to do the remaining checks if the version number is > not high enough. > Done > William |
From: Simon M. <sim...@sc...> - 2014-06-12 10:24:07
|
Can you also make the option names a bit more readable, I suggest changing: -addcflag => -addcflags -addldflag => -addldflags -addsrc => -addsources -vbl <level> => -buildverbosity <level> -intmod => -internalmodule -ol <library name> => -outputlibrary <name> Also OK Le 12/06/2014 12:22, Simon Marchetto a écrit : > > Le 12/06/2014 08:35, William S Fulton a écrit : >> On 10/06/14 16:54, Simon Marchetto wrote: >>> When do you want to make the release ? >>> >> As soon as the Scilab module is merged into master. Hopefully in a >> couple of weeks time, depending on what is found during review and if >> you have the time to address issues as they are found. >> >> >> I've read the Scilab.html chapter and have a few initial comments: >> >> Building the module section: >> - There is no ./scilab-cli script > Done => replaced by "scilab" >> - Can you describe more what the point of libexample.c is? Why is it >> needed in addition to the _wrap.c file? >> > Done => added the following short explanation "used by Scilab at run > time to link each module new declared function in Scilab to the > related wrapped C/C++function (ex: foo() in Scilab is routed to the > C/C++ function wrap_foo())" > > I will add at the end of the doc some other references to Scilab > documentation (describing Scilab external modules). You may find some > details there. >> Why is the file always called loader.sce ? In the other languages, >> you get a file named after the module name, so example_loader.sce >> would be more appropriate. Likewise for builder.sce. >> > Historical reason. I agree with you. I have to see if we can change it > in the future version of Scilab. >> The output of 'swig -scilab -help' does not match the documented >> command line options. Can you also format this output to line up like >> the other options? >> >> Can you also make the option names a bit more readable, I suggest >> changing: >> >> -addcflag => -addcflags >> -addldflag => -addldflags >> -addsrc => -addsources >> -vbl <level> => -buildverbosity <level> >> -intmod => -internalmodule >> -ol <library name> => -outputlibrary <name> >> >> "Note: in fact, it is not necessary to include the library >> typemaps.i, this one is included by default. " This is inconsistent >> with the other language, please change it to not include by default. >> > => I can run the example I gave in the documentation in Scilab and in > Python, without including typemaps.i. > Is it because this example does not need typemaps.i ? Or because > typemaps.i is included by default ? > In any case, I did not intend to include it by default. I did a search > now, I couldn't find any such include in Lib/scilab, but maybe I > missed it. > >> Unsigned integers are handled quite differently to signed integers >> wrt conversion to double. I don't see any logic in this, I think they >> should be handled the same, that is, unsigned and signed converted to >> and from doubles equally. Can you elaborate why this shouldn't be done? >> > OK I will fix it. >> Named typemaps use upper case as parameter names in SWIG. >> Can you modify the typemaps included by matrix.i to use upper case >> (and correct the html docs too). You could then drop the 'matrix' >> prefix, eg: >> matrixIn => IN >> > OK >> In the docs: "Scilab is supported from version 5.3.3". but >> configure.ac checks for 5.3 or higher. >> >> In configure: >> >> checking for scilab... scilab >> configure: Scilab version: 5.2.0.1266391513 >> checking for Scilab version is 5.3 or higher... no >> checking for Scilab startup options... -nwni -nb >> checking for Scilab header files... /usr/include >> > Done => configure checks 5.3.3 instead of 5.3. >> there is no need to do the remaining checks if the version number is >> not high enough. >> > Done > >> William > |
From: Simon M. <sim...@sc...> - 2014-06-16 15:33:15
|
I've done all of the changes. > Unsigned integers are handled quite differently to signed integers wrt > conversion to double. I don't see any logic in this, I think they > should be handled the same, that is, unsigned and signed converted to > and from doubles equally. Can you elaborate why this shouldn't be done? > Done > Named typemaps use upper case as parameter names in SWIG. > Can you modify the typemaps included by matrix.i to use upper case > (and correct the html docs too). You could then drop the 'matrix' > prefix, eg: > matrixIn => IN > Done Can you also make the option names a bit more readable, I suggest changing: > > -addcflag => -addcflags > -addldflag => -addldflags > -addsrc => -addsources > -vbl <level> => -buildverbosity <level> > -intmod => -internalmodule > -ol <library name> => -outputlibrary <name> > Done Simon > Le 12/06/2014 12:22, Simon Marchetto a écrit : >> >> Le 12/06/2014 08:35, William S Fulton a écrit : >>> On 10/06/14 16:54, Simon Marchetto wrote: >>>> When do you want to make the release ? >>>> >>> As soon as the Scilab module is merged into master. Hopefully in a >>> couple of weeks time, depending on what is found during review and >>> if you have the time to address issues as they are found. >>> >>> >>> I've read the Scilab.html chapter and have a few initial comments: >>> >>> Building the module section: >>> - There is no ./scilab-cli script >> Done => replaced by "scilab" >>> - Can you describe more what the point of libexample.c is? Why is it >>> needed in addition to the _wrap.c file? >>> >> Done => added the following short explanation "used by Scilab at run >> time to link each module new declared function in Scilab to the >> related wrapped C/C++function (ex: foo() in Scilab is routed to the >> C/C++ function wrap_foo())" >> >> I will add at the end of the doc some other references to Scilab >> documentation (describing Scilab external modules). You may find some >> details there. >>> Why is the file always called loader.sce ? In the other languages, >>> you get a file named after the module name, so example_loader.sce >>> would be more appropriate. Likewise for builder.sce. >>> >> Historical reason. I agree with you. I have to see if we can change >> it in the future version of Scilab. >>> The output of 'swig -scilab -help' does not match the documented >>> command line options. Can you also format this output to line up >>> like the other options? >>> >>> Can you also make the option names a bit more readable, I suggest >>> changing: >>> >>> -addcflag => -addcflags >>> -addldflag => -addldflags >>> -addsrc => -addsources >>> -vbl <level> => -buildverbosity <level> >>> -intmod => -internalmodule >>> -ol <library name> => -outputlibrary <name> >>> >>> "Note: in fact, it is not necessary to include the library >>> typemaps.i, this one is included by default. " This is inconsistent >>> with the other language, please change it to not include by default. >>> >> => I can run the example I gave in the documentation in Scilab and in >> Python, without including typemaps.i. >> Is it because this example does not need typemaps.i ? Or because >> typemaps.i is included by default ? >> In any case, I did not intend to include it by default. I did a >> search now, I couldn't find any such include in Lib/scilab, but maybe >> I missed it. >> >>> Unsigned integers are handled quite differently to signed integers >>> wrt conversion to double. I don't see any logic in this, I think >>> they should be handled the same, that is, unsigned and signed >>> converted to and from doubles equally. Can you elaborate why this >>> shouldn't be done? >>> >> OK I will fix it. >>> Named typemaps use upper case as parameter names in SWIG. >>> Can you modify the typemaps included by matrix.i to use upper case >>> (and correct the html docs too). You could then drop the 'matrix' >>> prefix, eg: >>> matrixIn => IN >>> >> OK >>> In the docs: "Scilab is supported from version 5.3.3". but >>> configure.ac checks for 5.3 or higher. >>> >>> In configure: >>> >>> checking for scilab... scilab >>> configure: Scilab version: 5.2.0.1266391513 >>> checking for Scilab version is 5.3 or higher... no >>> checking for Scilab startup options... -nwni -nb >>> checking for Scilab header files... /usr/include >>> >> Done => configure checks 5.3.3 instead of 5.3. >>> there is no need to do the remaining checks if the version number is >>> not high enough. >>> >> Done >> >>> William >> > |
From: William S F. <ws...@fu...> - 2014-06-20 06:58:25
|
On 16/06/14 16:33, Simon Marchetto wrote: > I've done all of the changes. > Great, thanks. >>>> "Note: in fact, it is not necessary to include the library >>>> typemaps.i, this one is included by default. " This is inconsistent >>>> with the other language, please change it to not include by default. >>>> >>> => I can run the example I gave in the documentation in Scilab and in >>> Python, without including typemaps.i. >>> Is it because this example does not need typemaps.i ? Or because >>> typemaps.i is included by default ? >>> In any case, I did not intend to include it by default. I did a >>> search now, I couldn't find any such include in Lib/scilab, but maybe >>> I missed it. On closer inspection the INPUT, INOUT, OUTPUT typemaps are included by default by some but not all the languages. I think that changed ages ago, so sorry, I'm very out of date. I've a few comments next on the build system... configure.ac: - scilab is not disabled if the header files are not found. - Does scilab not use pkg-config or a scilab-config? Most of the modern scripting language do so that it is easy to find the location of headers and binaries etc. Examples/Makefile.in: - uses abspath... please change to relative path - why do the scilab and scilab_cpp targets check for builder.sce? Don't the examples/test-suite always generate builder.sce? Does it have to set MAKEFLAGS, what is the default parallelisation for the builder? - the scilab_run target shouldn't have an if statement, it should always be run like the other languages - the use of eval and get_swig_scilab_args seems overly complex, can't it just use a simple if? - the swig executable invocation does not use -o - each of the individual example Makefiles sets TARGET to the name of the generated file, it should be the module name. - the contents of INCLUDES in test-suite/scilab/Makefile.in is different to normal, I'm not sure there is a good reason for this, please restore to normal Should Examples/scilab/matrix2/README be checked in? The constants, enum, template and variable examples have deviated from the versions in the other languages. Can you change them to use the identical source input files as say Java/Python. One day I would like common examples source for all different languages. William |
From: William S F. <ws...@fu...> - 2014-06-24 18:58:37
|
On 20/06/14 07:58, William S Fulton wrote: > On 16/06/14 16:33, Simon Marchetto wrote: > I've a few comments next on the build system... > > configure.ac: > - scilab is not disabled if the header files are not found. > - Does scilab not use pkg-config or a scilab-config? Most of the modern > scripting language do so that it is easy to find the location of headers > and binaries etc. > > Examples/Makefile.in: > - uses abspath... please change to relative path > - why do the scilab and scilab_cpp targets check for builder.sce? Don't > the examples/test-suite always generate builder.sce? Does it have to set > MAKEFLAGS, what is the default parallelisation for the builder? > - the scilab_run target shouldn't have an if statement, it should always > be run like the other languages > - the use of eval and get_swig_scilab_args seems overly complex, can't > it just use a simple if? > - the swig executable invocation does not use -o > - each of the individual example Makefiles sets TARGET to the name of > the generated file, it should be the module name. > - the contents of INCLUDES in test-suite/scilab/Makefile.in is different > to normal, I'm not sure there is a good reason for this, please restore > to normal > > Should Examples/scilab/matrix2/README be checked in? > > The constants, enum, template and variable examples have deviated from > the versions in the other languages. Can you change them to use the > identical source input files as say Java/Python. One day I would like > common examples source for all different languages. > Hi Simon Any response to the above points? I have some new comments... a) I suggest you change using 999 in Scierror to a macro, such as: #ifndef SWIG_SCILAB_ERROR #define SWIG_SCILAB_ERROR 900 #endif so that users can easily modify the error value via -D or %begin. The value you have chosen could all too easily be common value. How about using the SWIG error codes too from a base value of say 900 and use: SWIG_Scilab_Error(int code, const char *msg) { Scierror(SWIG_SCILAB_ERROR - code, _("SWIG/Scilab: %s: %s\n"), SWIG_Scilab_ErrorType(code), msg); } b) Parameter variable names are using _ as a prefix in the runtime code, please remove the prefix, it isn't a convention we use. c) What is the point of the multiple %rename in the li_std_string.i testcase? Is this because Scilab can't handle very long identifiers? d) Parameter variable names begin with _ in the Lib/scilab/* files. Can you drop the leading underscore please? By convention we only use this convention for member variables (and even then, not very often). e) Can you change the following fragment name: %fragment("include_for_uintptr", "header") { %#include <stdint.h> } to be "<stdint.h>" as this is how we name the fragments which are for #include. f) Can you add in the following type of lines in .travis.yml: ["scilab"]="-Werror -std=gnu89 -pedantic -fdiagnostics-show-option -Wno-long-long -Wreturn-type" and fix any warnings so it works (you may drop -pedantic if there are problems in the scilab headers): g) I think Lib/scilab/boost_shared_ptr.i is missing parts of a fix that went into 8c24e2ca749a23184ea32696201a26bce711ff5f (out and varout typemap changes) h) Can you change stl.i to pull in the same set of headers as the other languages, no more, no less! i) swig_this and swig_ptr are functions and should hence use SWIG instead of swig as the prefix. I'm nearing the end of my review. I'll probably post one more set of comments. William |
From: Simon M. <sim...@sc...> - 2014-06-25 15:09:05
|
Hi William, Sorry for delay, here are my answers. > configure.ac: >> - scilab is not disabled if the header files are not found. OK >> - Does scilab not use pkg-config or a scilab-config? Most of the modern >> scripting language do so that it is easy to find the location of headers >> and binaries etc. It seems we support pkg-config. I will use it if possible. >> >> Examples/Makefile.in: >> - uses abspath... please change to relative path Scilab changes of directory when it builds (with make).... If I give him a relative path, the compilation fails. I don't like this too, but I don't really have the choice. >> - why do the scilab and scilab_cpp targets check for builder.sce? Don't >> the examples/test-suite always generate builder.sce? Does it have to set >> MAKEFLAGS, what is the default parallelisation for the builder? We can remove the check. I have to override the MAKEFLAGS, otherwise the Scilab make is done in parallel and fails. I wrote some emails about this when I integrated Scilab into Travis. Despite my efforts, I could not find other way. >> - the scilab_run target shouldn't have an if statement, it should always >> be run like the other languages OK >> - the use of eval and get_swig_scilab_args seems overly complex, can't >> it just use a simple if? I hate duplicate code, but I can replace it by some ifs in targets scilab and scilab_cpp. These duplicate ifs won't be far each other, so it is not so hard to check. >> - the swig executable invocation does not use -o Need a little analysis on this. >> - each of the individual example Makefiles sets TARGET to the name of >> the generated file, it should be the module name. Need to see this also >> - the contents of INCLUDES in test-suite/scilab/Makefile.in is different >> to normal, I'm not sure there is a good reason for this, please restore >> to normal >> >> Should Examples/scilab/matrix2/README be checked in? It will be removed. >> >> The constants, enum, template and variable examples have deviated from >> the versions in the other languages. Can you change them to use the >> identical source input files as say Java/Python. One day I would like >> common examples source for all different languages. >> I understand this for consistency. But examples should also illustrate specific features of each language, shouldn't they ? Where can the user play with the feature with scilab_const, for example ? > Hi Simon > > Any response to the above points? I have some new comments... > > a) I suggest you change using 999 in Scierror to a macro, such as: > #ifndef SWIG_SCILAB_ERROR > #define SWIG_SCILAB_ERROR 900 > #endif > so that users can easily modify the error value via -D or %begin. The > value you have chosen could all too easily be common value. > How about using the SWIG error codes too from a base value of say 900 > and use: > SWIG_Scilab_Error(int code, const char *msg) > { > Scierror(SWIG_SCILAB_ERROR - code, _("SWIG/Scilab: %s: %s\n"), > SWIG_Scilab_ErrorType(code), msg); > } Error codes are defined in Scilab headers. 999 is a common error code in Scilab, but should be replaced by a macro. And I agree that the user should choose the error code value to discriminate between SWIG and Scilab. > > b) Parameter variable names are using _ as a prefix in the runtime > code, please remove the prefix, it isn't a convention we use. > OK > c) What is the point of the multiple %rename in the li_std_string.i > testcase? Is this because Scilab can't handle very long identifiers? > Surely ! > d) Parameter variable names begin with _ in the Lib/scilab/* files. > Can you drop the leading underscore please? By convention we only use > this convention for member variables (and even then, not very often). > OK > e) Can you change the following fragment name: > %fragment("include_for_uintptr", "header") { > %#include <stdint.h> > } > to be "<stdint.h>" as this is how we name the fragments which are for > #include. > OK > f) Can you add in the following type of lines in .travis.yml: > ["scilab"]="-Werror -std=gnu89 -pedantic > -fdiagnostics-show-option -Wno-long-long -Wreturn-type" > and fix any warnings so it works (you may drop -pedantic if there are > problems in the scilab headers): > OK, I'll check this. > g) I think Lib/scilab/boost_shared_ptr.i is missing parts of a fix > that went into 8c24e2ca749a23184ea32696201a26bce711ff5f (out and > varout typemap changes) In fact, I didn't have a look on Boost shared pointers at all. I'll check this fix. > > h) Can you change stl.i to pull in the same set of headers as the > other languages, no more, no less! > Even std::deque, std::multiset, etc... do work and are tested, in the case of Scilab ? OK > i) swig_this and swig_ptr are functions and should hence use SWIG > instead of swig as the prefix. > I picked up "swig_this" name from another language (Octave). OK > I'm nearing the end of my review. I'll probably post one more set of > comments. Good news ! And thanks for your review. > > William |
From: William S F. <ws...@fu...> - 2014-06-26 19:50:40
|
On 25/06/14 16:08, Simon Marchetto wrote: >>> >>> The constants, enum, template and variable examples have deviated from >>> the versions in the other languages. Can you change them to use the >>> identical source input files as say Java/Python. One day I would like >>> common examples source for all different languages. >>> > I understand this for consistency. > But examples should also illustrate specific features of each language, > shouldn't they ? > Where can the user play with the feature with scilab_const, for example ? > If you want scilab specific features, then please use another example. Also ask yourself it it really needs to be an example given the test-suite is what should be used for regressions. Too many examples are overwhelming for users in my opinion. >> c) What is the point of the multiple %rename in the li_std_string.i >> testcase? Is this because Scilab can't handle very long identifiers? >> > Surely ! A comment in the test-case would be helpful. >> h) Can you change stl.i to pull in the same set of headers as the >> other languages, no more, no less! >> > Even std::deque, std::multiset, etc... do work and are tested, in the > case of Scilab ? OK > This also applies for some of the other languages, but consistency across the different languages is preferable. I've finished the review now and have just these: 1) Can you add the Swig banner to all generated files please, it is missing in a few of them. 2) Can you change the use of char arrays and sprintf in scilab.cxx with String and Printf 3) I don't think the WARN_LANG_IDENTIFIER warning in scilab.cxx should only be turned on by -Wextra, rather it should be turned on by default and I think the warning should display both the untruncated and truncated name, similar to what Ruby does with WARN_RUBY_WRONG_NAME. Also change it to a language specific warning, I suggest calling it WARN_SCILAB_TRUNCATED_NAME. I've just committed a variety of changes too. Let me know when you've addressed all the comments made to date and I'll then look at merging to master. Thanks William |
From: Simon M. <sim...@sc...> - 2014-07-03 07:13:25
|
OK, so the remaining tasks are: - the swig executable invocation does not use -o - each of the individual example Makefiles sets TARGET to the name of the generated file, it should be the module name. - the contents of INCLUDES in test-suite/scilab/Makefile.in is different to normal, I'm not sure there is a good reason for this, please restore to normal - I suggest you change using 999 in Scierror to a macro, such as: #ifndef SWIG_SCILAB_ERROR #define SWIG_SCILAB_ERROR 900 #endif so that users can easily modify the error value via -D or %begin. The value you have chosen could all too easily be common value. How about using the SWIG error codes too from a base value of say 900 and use: SWIG_Scilab_Error(int code, const char *msg) { Scierror(SWIG_SCILAB_ERROR - code, _("SWIG/Scilab: %s: %s\n"), SWIG_Scilab_ErrorType(code), msg); } - Parameter variable names are using _ as a prefix in the runtime code, please remove the prefix, it isn't a convention we use. - Can you add in the following type of lines in .travis.yml: ["scilab"]="-Werror -std=gnu89 -pedantic -fdiagnostics-show-option -Wno-long-long -Wreturn-type" and fix any warnings so it works (you may drop -pedantic if there are problems in the scilab headers): - I think Lib/scilab/boost_shared_ptr.i is missing parts of a fix that went into 8c24e2ca749a23184ea32696201a26bce711ff5f (out and varout typemap changes) - Can you change stl.i to pull in the same set of headers as the other languages, no more, no less! - rewrite unconsistent examples - Can you add the Swig banner to all generated files please, it is missing in a few of them. - Can you change the use of char arrays and sprintf in scilab.cxx with String and Printf - I don't think the WARN_LANG_IDENTIFIER warning in scilab.cxx should only be turned on by -Wextra, rather it should be turned on by default and I think the warning should display both the untruncated and truncated name, similar to what Ruby does with WARN_RUBY_WRONG_NAME. Also change it to a language specific warning, I suggest calling it WARN_SCILAB_TRUNCATED_NAME. I will let you know when I will finish adressing all these. Simon Le 26/06/2014 21:50, William S Fulton a écrit : > On 25/06/14 16:08, Simon Marchetto wrote: > >>>> >>>> The constants, enum, template and variable examples have deviated from >>>> the versions in the other languages. Can you change them to use the >>>> identical source input files as say Java/Python. One day I would like >>>> common examples source for all different languages. >>>> >> I understand this for consistency. >> But examples should also illustrate specific features of each language, >> shouldn't they ? >> Where can the user play with the feature with scilab_const, for >> example ? >> > If you want scilab specific features, then please use another example. > Also ask yourself it it really needs to be an example given the > test-suite is what should be used for regressions. Too many examples > are overwhelming for users in my opinion. > >>> c) What is the point of the multiple %rename in the li_std_string.i >>> testcase? Is this because Scilab can't handle very long identifiers? >>> >> Surely ! > A comment in the test-case would be helpful. > >>> h) Can you change stl.i to pull in the same set of headers as the >>> other languages, no more, no less! >>> >> Even std::deque, std::multiset, etc... do work and are tested, in the >> case of Scilab ? OK >> > This also applies for some of the other languages, but consistency > across the different languages is preferable. > > I've finished the review now and have just these: > > 1) Can you add the Swig banner to all generated files please, it is > missing in a few of them. > 2) Can you change the use of char arrays and sprintf in scilab.cxx > with String and Printf > 3) I don't think the WARN_LANG_IDENTIFIER warning in scilab.cxx should > only be turned on by -Wextra, rather it should be turned on by default > and I think the warning should display both the untruncated and > truncated name, similar to what Ruby does with WARN_RUBY_WRONG_NAME. > Also change it to a language specific warning, I suggest calling it > WARN_SCILAB_TRUNCATED_NAME. > > I've just committed a variety of changes too. Let me know when you've > addressed all the comments made to date and I'll then look at merging > to master. > > Thanks > William > |
From: Simon M. <sim...@sc...> - 2014-08-08 16:33:12
|
Hello, I have fixed all the remaining issues. For most of them, I follow your indications, for a few I could'nt. Below you can find the details. I am having now a last look on the code and the documentation. For example checking the coding style. About this : documentation says to apply K&R style on C/C++ module source code. But on typemaps source code ? Simon Le 03/07/2014 09:13, Simon Marchetto a écrit : - the swig executable invocation does not use -o Done - each of the individual example Makefiles sets TARGET to the name of the generated file, it should be the module name.Done Done - the contents of INCLUDES in test-suite/scilab/Makefile.in is different to normal, I'm not sure there is a good reason for this, please restore to normal I overloaded the INCLUDES because each test stuff is generated in a specific sub folder. Some tests cannot compile. I could not find simpler way to fix this. - I suggest you change using 999 in Scierror to a macro, such as: #ifndef SWIG_SCILAB_ERROR #define SWIG_SCILAB_ERROR 900 #endif so that users can easily modify the error value via -D or %begin. The value you have chosen could all too easily be common value. How about using the SWIG error codes too from a base value of say 900 and use: SWIG_Scilab_Error(int code, const char *msg) { Scierror(SWIG_SCILAB_ERROR - code, _("SWIG/Scilab: %s: %s\n"), SWIG_Scilab_ErrorType(code), msg); } 999 is a Scilab API error code widely used, it has been introduced long time ago, but unfortunately with no macro associated. At last, I defined the macro in SWIG (SWIG_API_ARGUMENT_ERROR). As it is a Scilab API error code, this macro is not aimed to be changed. But some of 999 errors codes were used for SWIG errors and I follow your advices and changed it to SWIG_ASCILAB_ERROR. - Parameter variable names are using _ as a prefix in the runtime code, please remove the prefix, it isn't a convention we use. Done - Can you add in the following type of lines in .travis.yml: ["scilab"]="-Werror -std=gnu89 -pedantic -fdiagnostics-show-option -Wno-long-long -Wreturn-type" and fix any warnings so it works (you may drop -pedantic if there are problems in the scilab headers): Done - I think Lib/scilab/boost_shared_ptr.i is missing parts of a fix that went into 8c24e2ca749a23184ea32696201a26bce711ff5f (out and varout typemap changes) Done - Can you change stl.i to pull in the same set of headers as the other languages, no more, no less! Done - rewrite unconsistent examples Done - Can you add the Swig banner to all generated files please, it is missing in a few of them. Which files ? There are files like loader.sce with no SWIG banner, but they are generated by Scilab. - Can you change the use of char arrays and sprintf in scilab.cxx with String and Printf Done - I don't think the WARN_LANG_IDENTIFIER warning in scilab.cxx should only be turned on by -Wextra, rather it should be turned on by default and I think the warning should display both the untruncated and truncated name, similar to what Ruby does with WARN_RUBY_WRONG_NAME. Also change it to a language specific warning, I suggest calling it WARN_SCILAB_TRUNCATED_NAME. Done |
From: Simon M. <sim...@sc...> - 2014-06-11 10:02:06
|
It seems the broken test may be a regression on nested structs. I ve created an issue #1373 <https://sourceforge.net/p/swig/bugs/1373/>, so that you can have a look at it. Simon Le 10/06/2014 17:54, Simon Marchetto a écrit : > William, > > I had indeed some troubles with the out of source build but it works now. > I have a broken test to fix, and a couple of things to check and do, > but it won't take long. > > When do you want to make the release ? > > Simon > > Le 06/06/2014 15:44, Simon Marchetto a écrit : >> With pleasure :-) I have begun to merge. >> >> I don't think the "out of sources build" changes will be hard to >> integrate, but we'll see. >> >> Simon >> >> Le 06/06/2014 08:51, William S Fulton a écrit : >>> Iwill be focusing next on the Scilab integration into master. >>> >>> Simon, can you please merge master into the gsoc2012-scilab branch ? >>> Since your last merge from master in March, there have unfortunately >>> (for you) been a number of build changeswhich you will need to >>> review and make sure is up to date on your branchbefore we can merge >>> to master. >>> >>> - Please look at all the makefile changes and replicate what has >>> happened on master into your files. The changes are aimed at >>> providing out of source builds and a better clean. >>> - Please review your .gitignore changes and makesure the master >>> version does not have equivalentsto your changes. >>> - Note that the latest Travis testing might break as it now does out >>> of source builds and checks that make maintainer-clean is working >>> (ie no unexpected files leftafter a clean). >>> >>> I'll be posting further review comments to this email thread. >>> >>> William >>> >>> >>> ------------------------------------------------------------------------------ >>> Learn Graph Databases - Download FREE O'Reilly Book >>> "Graph Databases" is the definitive new guide to graph databases and their >>> applications. Written by three acclaimed leaders in the field, >>> this first edition is now available. Download your free book today! >>> http://p.sf.net/sfu/NeoTech >>> >>> >>> _______________________________________________ >>> Swig-devel mailing list >>> Swi...@li... >>> https://lists.sourceforge.net/lists/listinfo/swig-devel >> >> >> >> ------------------------------------------------------------------------------ >> Learn Graph Databases - Download FREE O'Reilly Book >> "Graph Databases" is the definitive new guide to graph databases and their >> applications. Written by three acclaimed leaders in the field, >> this first edition is now available. Download your free book today! >> http://p.sf.net/sfu/NeoTech >> >> >> _______________________________________________ >> Swig-devel mailing list >> Swi...@li... >> https://lists.sourceforge.net/lists/listinfo/swig-devel > |
From: William S F. <ws...@fu...> - 2014-06-12 06:48:52
|
Yes, this looks like a nested struct regression. Vladimir, would you kindly take a look? William On 11/06/14 11:02, Simon Marchetto wrote: > It seems the broken test may be a regression on nested structs. > I ve created an issue #1373 <https://sourceforge.net/p/swig/bugs/1373/>, > so that you can have a look at it. > > Simon > > > Le 10/06/2014 17:54, Simon Marchetto a écrit : >> William, >> >> I had indeed some troubles with the out of source build but it works now. >> I have a broken test to fix, and a couple of things to check and do, >> but it won't take long. >> >> When do you want to make the release ? >> >> Simon >> >> Le 06/06/2014 15:44, Simon Marchetto a écrit : >>> With pleasure :-) I have begun to merge. >>> >>> I don't think the "out of sources build" changes will be hard to >>> integrate, but we'll see. >>> >>> Simon >>> >>> Le 06/06/2014 08:51, William S Fulton a écrit : >>>> Iwill be focusing next on the Scilab integration into master. >>>> >>>> Simon, can you please merge master into the gsoc2012-scilab branch ? >>>> Since your last merge from master in March, there have unfortunately >>>> (for you) been a number of build changeswhich you will need to >>>> review and make sure is up to date on your branchbefore we can merge >>>> to master. >>>> >>>> - Please look at all the makefile changes and replicate what has >>>> happened on master into your files. The changes are aimed at >>>> providing out of source builds and a better clean. >>>> - Please review your .gitignore changes and makesure the master >>>> version does not have equivalentsto your changes. >>>> - Note that the latest Travis testing might break as it now does out >>>> of source builds and checks that make maintainer-clean is working >>>> (ie no unexpected files leftafter a clean). >>>> >>>> I'll be posting further review comments to this email thread. >>>> >>>> William >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Learn Graph Databases - Download FREE O'Reilly Book >>>> "Graph Databases" is the definitive new guide to graph databases and their >>>> applications. Written by three acclaimed leaders in the field, >>>> this first edition is now available. Download your free book today! >>>> http://p.sf.net/sfu/NeoTech >>>> >>>> >>>> _______________________________________________ >>>> Swig-devel mailing list >>>> Swi...@li... >>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>> >>> >>> >>> ------------------------------------------------------------------------------ >>> Learn Graph Databases - Download FREE O'Reilly Book >>> "Graph Databases" is the definitive new guide to graph databases and their >>> applications. Written by three acclaimed leaders in the field, >>> this first edition is now available. Download your free book today! >>> http://p.sf.net/sfu/NeoTech >>> >>> >>> _______________________________________________ >>> Swig-devel mailing list >>> Swi...@li... >>> https://lists.sourceforge.net/lists/listinfo/swig-devel >> > > > > ------------------------------------------------------------------------------ > HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions > Find What Matters Most in Your Big Data with HPCC Systems > Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. > Leverages Graph Analysis for Fast Processing & Easy Data Exploration > http://p.sf.net/sfu/hpccsystems > > > > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel > |
From: Vladimir K. <vka...@op...> - 2014-06-15 21:04:09
|
Hi William, I don't quite agree that the issue #1373 is a bug. In the case quoted in the bug description: struct Outer { struct { int val; } inner1, inner2; }; Outer.inner1 and Outer.inner2 both have the same type (unnamed structure), they do not represent 2 different structures. When this unnamed structure gets a name (Outer_inner1 in this case) both Outer.inner1 and Outer.inner2 accessors return Outer_inner1 value in the scripting language. (If the types would be different it would be impossible to e.g. assign Outer.inner1 to Outer.inner2 w/o a cast.) Best regards, Vladimir mailto:VKa...@op... -----Original Message----- Yes, this looks like a nested struct regression. Vladimir, would you kindly take a look? William On 11/06/14 11:02, Simon Marchetto wrote: > It seems the broken test may be a regression on nested structs. > I ve created an issue #1373 <https://sourceforge.net/p/swig/bugs/1373/>, > so that you can have a look at it. > > Simon > > > Le 10/06/2014 17:54, Simon Marchetto a écrit : >> William, >> >> I had indeed some troubles with the out of source build but it works now. >> I have a broken test to fix, and a couple of things to check and do, >> but it won't take long. >> >> When do you want to make the release ? >> >> Simon >> >> Le 06/06/2014 15:44, Simon Marchetto a écrit : >>> With pleasure :-) I have begun to merge. >>> >>> I don't think the "out of sources build" changes will be hard to >>> integrate, but we'll see. >>> >>> Simon >>> >>> Le 06/06/2014 08:51, William S Fulton a écrit : >>>> Iwill be focusing next on the Scilab integration into master. >>>> >>>> Simon, can you please merge master into the gsoc2012-scilab branch ? >>>> Since your last merge from master in March, there have unfortunately >>>> (for you) been a number of build changeswhich you will need to >>>> review and make sure is up to date on your branchbefore we can merge >>>> to master. >>>> >>>> - Please look at all the makefile changes and replicate what has >>>> happened on master into your files. The changes are aimed at >>>> providing out of source builds and a better clean. >>>> - Please review your .gitignore changes and makesure the master >>>> version does not have equivalentsto your changes. >>>> - Note that the latest Travis testing might break as it now does out >>>> of source builds and checks that make maintainer-clean is working >>>> (ie no unexpected files leftafter a clean). >>>> >>>> I'll be posting further review comments to this email thread. >>>> >>>> William >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Learn Graph Databases - Download FREE O'Reilly Book >>>> "Graph Databases" is the definitive new guide to graph databases and their >>>> applications. Written by three acclaimed leaders in the field, >>>> this first edition is now available. Download your free book today! >>>> http://p.sf.net/sfu/NeoTech >>>> >>>> >>>> _______________________________________________ >>>> Swig-devel mailing list >>>> Swi...@li... >>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>> >>> >>> >>> ------------------------------------------------------------------------------ >>> Learn Graph Databases - Download FREE O'Reilly Book >>> "Graph Databases" is the definitive new guide to graph databases and their >>> applications. Written by three acclaimed leaders in the field, >>> this first edition is now available. Download your free book today! >>> http://p.sf.net/sfu/NeoTech >>> >>> >>> _______________________________________________ >>> Swig-devel mailing list >>> Swi...@li... >>> https://lists.sourceforge.net/lists/listinfo/swig-devel >> > > > > ------------------------------------------------------------------------------ > HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions > Find What Matters Most in Your Big Data with HPCC Systems > Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. > Leverages Graph Analysis for Fast Processing & Easy Data Exploration > http://p.sf.net/sfu/hpccsystems > > > > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel > ----- No virus found in this message. Checked by AVG - www.avg.com Version: 2012.0.2247 / Virus Database: 3955/7179 - Release Date: 06/15/14 |
From: William S F. <ws...@fu...> - 2014-06-18 19:05:33
|
Vladimir, you are absolutely right. Sorry, I misunderstood and thought it wasn't generating the accessors for inner2. Simon, you need to change your test to call Outer_inner1_val_get() and pass either an instance of inner1 or inner2. William On 15/06/14 22:03, Vladimir Kalinin wrote: > Hi William, > > I don't quite agree that the issue #1373 is a bug. > > In the case quoted in the bug description: > > struct Outer { > struct { > int val; > } inner1, inner2; > }; > > Outer.inner1 and Outer.inner2 both have the same type (unnamed > structure), they do not represent 2 different structures. > When this unnamed structure gets a name (Outer_inner1 in this case) > both Outer.inner1 and Outer.inner2 accessors return Outer_inner1 value in the > scripting language. (If the types would be different it would be > impossible to e.g. assign Outer.inner1 to Outer.inner2 w/o a cast.) > > Best regards, > Vladimir mailto:VKa...@op... > > -----Original Message----- > Yes, this looks like a nested struct regression. Vladimir, would you > kindly take a look? > > William > > On 11/06/14 11:02, Simon Marchetto wrote: >> It seems the broken test may be a regression on nested structs. >> I ve created an issue #1373 <https://sourceforge.net/p/swig/bugs/1373/>, >> so that you can have a look at it. >> >> Simon >> >> >> Le 10/06/2014 17:54, Simon Marchetto a écrit : >>> William, >>> >>> I had indeed some troubles with the out of source build but it works now. >>> I have a broken test to fix, and a couple of things to check and do, >>> but it won't take long. >>> >>> When do you want to make the release ? >>> >>> Simon >>> >>> Le 06/06/2014 15:44, Simon Marchetto a écrit : >>>> With pleasure :-) I have begun to merge. >>>> >>>> I don't think the "out of sources build" changes will be hard to >>>> integrate, but we'll see. >>>> >>>> Simon >>>> >>>> Le 06/06/2014 08:51, William S Fulton a écrit : >>>>> Iwill be focusing next on the Scilab integration into master. >>>>> >>>>> Simon, can you please merge master into the gsoc2012-scilab branch ? >>>>> Since your last merge from master in March, there have unfortunately >>>>> (for you) been a number of build changeswhich you will need to >>>>> review and make sure is up to date on your branchbefore we can merge >>>>> to master. >>>>> >>>>> - Please look at all the makefile changes and replicate what has >>>>> happened on master into your files. The changes are aimed at >>>>> providing out of source builds and a better clean. >>>>> - Please review your .gitignore changes and makesure the master >>>>> version does not have equivalentsto your changes. >>>>> - Note that the latest Travis testing might break as it now does out >>>>> of source builds and checks that make maintainer-clean is working >>>>> (ie no unexpected files leftafter a clean). >>>>> >>>>> I'll be posting further review comments to this email thread. >>>>> >>>>> William >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> Learn Graph Databases - Download FREE O'Reilly Book >>>>> "Graph Databases" is the definitive new guide to graph databases and their >>>>> applications. Written by three acclaimed leaders in the field, >>>>> this first edition is now available. Download your free book today! >>>>> http://p.sf.net/sfu/NeoTech >>>>> >>>>> >>>>> _______________________________________________ >>>>> Swig-devel mailing list >>>>> Swi...@li... >>>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Learn Graph Databases - Download FREE O'Reilly Book >>>> "Graph Databases" is the definitive new guide to graph databases and their >>>> applications. Written by three acclaimed leaders in the field, >>>> this first edition is now available. Download your free book today! >>>> http://p.sf.net/sfu/NeoTech >>>> >>>> >>>> _______________________________________________ >>>> Swig-devel mailing list >>>> Swi...@li... >>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>> >> >> >> >> ------------------------------------------------------------------------------ >> HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions >> Find What Matters Most in Your Big Data with HPCC Systems >> Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. >> Leverages Graph Analysis for Fast Processing & Easy Data Exploration >> http://p.sf.net/sfu/hpccsystems >> >> >> >> _______________________________________________ >> Swig-devel mailing list >> Swi...@li... >> https://lists.sourceforge.net/lists/listinfo/swig-devel >> > > > > ----- > No virus found in this message. > Checked by AVG - www.avg.com > Version: 2012.0.2247 / Virus Database: 3955/7179 - Release Date: 06/15/14 > > |
From: Simon M. <sim...@sc...> - 2014-06-20 07:46:54
|
I changed the test. I have to fix the clean and the test-suite will be ok again. Le 18/06/2014 21:05, William S Fulton a écrit : > Vladimir, you are absolutely right. Sorry, I misunderstood and thought > it wasn't generating the accessors for inner2. > > Simon, you need to change your test to call Outer_inner1_val_get() and > pass either an instance of inner1 or inner2. > > William > > On 15/06/14 22:03, Vladimir Kalinin wrote: >> Hi William, >> >> I don't quite agree that the issue #1373 is a bug. >> >> In the case quoted in the bug description: >> >> struct Outer { >> struct { >> int val; >> } inner1, inner2; >> }; >> >> Outer.inner1 and Outer.inner2 both have the same type (unnamed >> structure), they do not represent 2 different structures. >> When this unnamed structure gets a name (Outer_inner1 in this case) >> both Outer.inner1 and Outer.inner2 accessors return Outer_inner1 >> value in the >> scripting language. (If the types would be different it would be >> impossible to e.g. assign Outer.inner1 to Outer.inner2 w/o a cast.) >> >> Best regards, >> Vladimir mailto:VKa...@op... >> >> -----Original Message----- >> Yes, this looks like a nested struct regression. Vladimir, would you >> kindly take a look? >> >> William >> >> On 11/06/14 11:02, Simon Marchetto wrote: >>> It seems the broken test may be a regression on nested structs. >>> I ve created an issue #1373 >>> <https://sourceforge.net/p/swig/bugs/1373/>, >>> so that you can have a look at it. >>> >>> Simon >>> >>> >>> Le 10/06/2014 17:54, Simon Marchetto a écrit : >>>> William, >>>> >>>> I had indeed some troubles with the out of source build but it >>>> works now. >>>> I have a broken test to fix, and a couple of things to check and do, >>>> but it won't take long. >>>> >>>> When do you want to make the release ? >>>> >>>> Simon >>>> >>>> Le 06/06/2014 15:44, Simon Marchetto a écrit : >>>>> With pleasure :-) I have begun to merge. >>>>> >>>>> I don't think the "out of sources build" changes will be hard to >>>>> integrate, but we'll see. >>>>> >>>>> Simon >>>>> >>>>> Le 06/06/2014 08:51, William S Fulton a écrit : >>>>>> Iwill be focusing next on the Scilab integration into master. >>>>>> >>>>>> Simon, can you please merge master into the gsoc2012-scilab branch ? >>>>>> Since your last merge from master in March, there have unfortunately >>>>>> (for you) been a number of build changeswhich you will need to >>>>>> review and make sure is up to date on your branchbefore we can merge >>>>>> to master. >>>>>> >>>>>> - Please look at all the makefile changes and replicate what has >>>>>> happened on master into your files. The changes are aimed at >>>>>> providing out of source builds and a better clean. >>>>>> - Please review your .gitignore changes and makesure the master >>>>>> version does not have equivalentsto your changes. >>>>>> - Note that the latest Travis testing might break as it now does out >>>>>> of source builds and checks that make maintainer-clean is working >>>>>> (ie no unexpected files leftafter a clean). >>>>>> >>>>>> I'll be posting further review comments to this email thread. >>>>>> >>>>>> William >>>>>> >>>>>> >>>>>> ------------------------------------------------------------------------------ >>>>>> >>>>>> Learn Graph Databases - Download FREE O'Reilly Book >>>>>> "Graph Databases" is the definitive new guide to graph databases >>>>>> and their >>>>>> applications. Written by three acclaimed leaders in the field, >>>>>> this first edition is now available. Download your free book today! >>>>>> http://p.sf.net/sfu/NeoTech >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> Swig-devel mailing list >>>>>> Swi...@li... >>>>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> >>>>> Learn Graph Databases - Download FREE O'Reilly Book >>>>> "Graph Databases" is the definitive new guide to graph databases >>>>> and their >>>>> applications. Written by three acclaimed leaders in the field, >>>>> this first edition is now available. Download your free book today! >>>>> http://p.sf.net/sfu/NeoTech >>>>> >>>>> >>>>> _______________________________________________ >>>>> Swig-devel mailing list >>>>> Swi...@li... >>>>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>>> >>> >>> >>> >>> ------------------------------------------------------------------------------ >>> >>> HPCC Systems Open Source Big Data Platform from LexisNexis Risk >>> Solutions >>> Find What Matters Most in Your Big Data with HPCC Systems >>> Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. >>> Leverages Graph Analysis for Fast Processing & Easy Data Exploration >>> http://p.sf.net/sfu/hpccsystems >>> >>> >>> >>> _______________________________________________ >>> Swig-devel mailing list >>> Swi...@li... >>> https://lists.sourceforge.net/lists/listinfo/swig-devel >>> >> >> >> >> ----- >> No virus found in this message. >> Checked by AVG - www.avg.com >> Version: 2012.0.2247 / Virus Database: 3955/7179 - Release Date: >> 06/15/14 >> >> > |
From: William S F. <ws...@fu...> - 2014-08-11 18:02:26
|
On 08/08/14 17:13, Simon Marchetto wrote: > Hello, > > I have fixed all the remaining issues. > For most of them, I follow your indications, for a few I could'nt. > Below you can find the details. Thanks, I'll look over them in the next few days. > > I am having now a last look on the code and the documentation. > For example checking the coding style. About this : documentation says > to apply K&R style on C/C++ module source code. But on typemaps source > code ? > Use the same style as in the SWIG source code, but using spaces is better than tabs because you don't know what the tab setting for users is set to. Can you let me know if you're going to make changes in this area? > > - Can you add the Swig banner to all generated files please, it is > missing in a few of them. > > > Which files ? There are files like loader.sce with no SWIG banner, but they are generated by Scilab. > Okay, that makes sense then. William |
From: Simon M. <sim...@sc...> - 2014-08-19 15:43:08
|
Hello, I did some minor modifications on braces for coding style, and a few fixes in documentation. I think this branch is ready and won't have any other changes in this summer :-), except if you ask. Regards, Simon Le 11/08/2014 20:02, William S Fulton a écrit : > On 08/08/14 17:13, Simon Marchetto wrote: >> Hello, >> >> I have fixed all the remaining issues. >> For most of them, I follow your indications, for a few I could'nt. >> Below you can find the details. > Thanks, I'll look over them in the next few days. >> >> I am having now a last look on the code and the documentation. >> For example checking the coding style. About this : documentation says >> to apply K&R style on C/C++ module source code. But on typemaps source >> code ? >> > Use the same style as in the SWIG source code, but using spaces is > better than tabs because you don't know what the tab setting for users > is set to. Can you let me know if you're going to make changes in this > area? > >> >> - Can you add the Swig banner to all generated files please, it is >> missing in a few of them. >> >> >> Which files ? There are files like loader.sce with no SWIG banner, >> but they are generated by Scilab. >> > Okay, that makes sense then. > > William |
From: Simon M. <sim...@sc...> - 2014-08-26 08:59:23
|
Hello, There is one more thing to do, fix the documentation chapter numbering. Scilab should take the place of Tcl (39), do you confirm this ? Simon Le 19/08/2014 17:43, Simon Marchetto a écrit : > Hello, > > I did some minor modifications on braces for coding style, and a few > fixes in documentation. > I think this branch is ready and won't have any other changes in this > summer :-), except if you ask. > > Regards, > > Simon > > > Le 11/08/2014 20:02, William S Fulton a écrit : >> On 08/08/14 17:13, Simon Marchetto wrote: >>> Hello, >>> >>> I have fixed all the remaining issues. >>> For most of them, I follow your indications, for a few I could'nt. >>> Below you can find the details. >> Thanks, I'll look over them in the next few days. >>> I am having now a last look on the code and the documentation. >>> For example checking the coding style. About this : documentation says >>> to apply K&R style on C/C++ module source code. But on typemaps source >>> code ? >>> >> Use the same style as in the SWIG source code, but using spaces is >> better than tabs because you don't know what the tab setting for users >> is set to. Can you let me know if you're going to make changes in this >> area? >> >>> - Can you add the Swig banner to all generated files please, it is >>> missing in a few of them. >>> >>> >>> Which files ? There are files like loader.sce with no SWIG banner, >>> but they are generated by Scilab. >>> >> Okay, that makes sense then. >> >> William > > ------------------------------------------------------------------------------ > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel |
From: William S F. <ws...@fu...> - 2014-08-28 18:46:16
|
It'll be added in alphabetical order so yes, it will go before Tcl. Please don't make make any html chapter numbering changes, I'll do this after the merge. Incidentally, I am looking at your recent changes now. William On 26 August 2014 09:59, Simon Marchetto < sim...@sc...> wrote: > Hello, > > There is one more thing to do, fix the documentation chapter numbering. > Scilab should take the place of Tcl (39), do you confirm this ? > > Simon > > |
From: William S F. <ws...@fu...> - 2014-08-28 21:56:26
|
It'll be added in alphabetical order so yes, it will go before Tcl. Please don't make make any html chapter numbering changes, I'll do this after the merge. Incidentally, I am looking at your recent changes now. William Hello, There is one more thing to do, fix the documentation chapter numbering. Scilab should take the place of Tcl (39), do you confirm this ? Simon Le 19/08/2014 17:43, Simon Marchetto a écrit : > Hello, > > I did some minor modifications on braces for coding style, and a few > fixes in documentation. > I think this branch is ready and won't have any other changes in this > summer :-), except if you ask. > > Regards, > > Simon > > > Le 11/08/2014 20:02, William S Fulton a écrit : >> On 08/08/14 17:13, Simon Marchetto wrote: >>> Hello, >>> >>> I have fixed all the remaining issues. >>> For most of them, I follow your indications, for a few I could'nt. >>> Below you can find the details. >> Thanks, I'll look over them in the next few days. >>> I am having now a last look on the code and the documentation. >>> For example checking the coding style. About this : documentation says >>> to apply K&R style on C/C++ module source code. But on typemaps source >>> code ? >>> >> Use the same style as in the SWIG source code, but using spaces is >> better than tabs because you don't know what the tab setting for users >> is set to. Can you let me know if you're going to make changes in this >> area? >> >>> - Can you add the Swig banner to all generated files please, it is >>> missing in a few of them. >>> >>> >>> Which files ? There are files like loader.sce with no SWIG banner, >>> but they are generated by Scilab. >>> >> Okay, that makes sense then. >> >> William > > ------------------------------------------------------------------------------ > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ Swig-devel mailing list Swi...@li... https://lists.sourceforge.net/lists/listinfo/swig-devel |
From: William S F. <ws...@fu...> - 2014-09-04 19:48:01
|
I've made some tweaks and have tested using scilab-5.5.0 and the test-suite and examples pass. These are the current problems... Can you fix this warning in the matrix2 and std_list examples: Warning : redefining function: squareDoubleMatrix . Use funcprot(0) to avoid this message Can you these warnings in the constants exammple: SWIG/Scilab: RuntimeError: Contract violation: require: (arg1>=0)&&(arg2>=0) SWIG/Scilab: RuntimeError: Contract violation: require: (arg1>=0) output is not used in this function, is this intentional? SWIG_Scilab_SetOutput(void *pvApiCtx, SwigSciObject output) { https://travis-ci.org/swig/swig/jobs/34321998 should have failed the build but didn't: 1) What is the problem here in the li_std_vector test? Changing the name of a global variable shouldn't make it fail. 2) I can't diagnose this problem using normal compiler techniques like g++ -E. When are you going to get rid of builder.sce and will this mean that we will have a normal sane way to build Scilab wrappers simply using the compiler like other languages? 3) The failure should break the build, but it passed! I think if you can drop builder.sce and use normal compile commands, as it will remove the extra complexity. William On 28 August 2014 22:56, William S Fulton <ws...@fu...> wrote: > > It'll be added in alphabetical order so yes, it will go before Tcl. Please > don't make make any html chapter numbering changes, I'll do this after the > merge. Incidentally, I am looking at your recent changes now. > > William > Hello, > > > There is one more thing to do, fix the documentation chapter numbering. > Scilab should take the place of Tcl (39), do you confirm this ? > > Simon > > Le 19/08/2014 17:43, Simon Marchetto a écrit : > > Hello, > > > > I did some minor modifications on braces for coding style, and a few > > fixes in documentation. > > I think this branch is ready and won't have any other changes in this > > summer :-), except if you ask. > > > > Regards, > > > > Simon > > > > > > Le 11/08/2014 20:02, William S Fulton a écrit : > >> On 08/08/14 17:13, Simon Marchetto wrote: > >>> Hello, > >>> > >>> I have fixed all the remaining issues. > >>> For most of them, I follow your indications, for a few I could'nt. > >>> Below you can find the details. > >> Thanks, I'll look over them in the next few days. > >>> I am having now a last look on the code and the documentation. > >>> For example checking the coding style. About this : documentation says > >>> to apply K&R style on C/C++ module source code. But on typemaps source > >>> code ? > >>> > >> Use the same style as in the SWIG source code, but using spaces is > >> better than tabs because you don't know what the tab setting for users > >> is set to. Can you let me know if you're going to make changes in this > >> area? > >> > >>> - Can you add the Swig banner to all generated files please, it is > >>> missing in a few of them. > >>> > >>> > >>> Which files ? There are files like loader.sce with no SWIG banner, > >>> but they are generated by Scilab. > >>> > >> Okay, that makes sense then. > >> > >> William > > > > > ------------------------------------------------------------------------------ > > _______________________________________________ > > Swig-devel mailing list > > Swi...@li... > > https://lists.sourceforge.net/lists/listinfo/swig-devel > > > > ------------------------------------------------------------------------------ > Slashdot TV. > Video for Nerds. Stuff that matters. > http://tv.slashdot.org/ > _______________________________________________ > Swig-devel mailing list > Swi...@li... > https://lists.sourceforge.net/lists/listinfo/swig-devel > |
From: William S F. <ws...@fu...> - 2014-09-04 20:00:06
|
I also meant to mention that when using -o including a sub-directory in the name, then I can't get anything to build. Is this another builderr.sce quirk/bug?. William On 4 September 2014 20:47, William S Fulton <ws...@fu...> wrote: > I've made some tweaks and have tested using scilab-5.5.0 and the > test-suite and examples pass. These are the current problems... > > Can you fix this warning in the matrix2 and std_list examples: > Warning : redefining function: squareDoubleMatrix . Use funcprot(0) > to avoid this message > > Can you these warnings in the constants exammple: > SWIG/Scilab: RuntimeError: Contract violation: require: > (arg1>=0)&&(arg2>=0) > SWIG/Scilab: RuntimeError: Contract violation: require: (arg1>=0) > > output is not used in this function, is this intentional? > SWIG_Scilab_SetOutput(void *pvApiCtx, SwigSciObject output) { > > https://travis-ci.org/swig/swig/jobs/34321998 should have failed the > build but didn't: > 1) What is the problem here in the li_std_vector test? Changing the name > of a global variable shouldn't make it fail. > 2) I can't diagnose this problem using normal compiler techniques like g++ > -E. When are you going to get rid of builder.sce and will this mean that we > will have a normal sane way to build Scilab wrappers simply using the > compiler like other languages? > 3) The failure should break the build, but it passed! I think if you can > drop builder.sce and use normal compile commands, as it will remove the > extra complexity. > > William > > > > On 28 August 2014 22:56, William S Fulton <ws...@fu...> wrote: > >> >> It'll be added in alphabetical order so yes, it will go before Tcl. >> Please don't make make any html chapter numbering changes, I'll do this >> after the merge. Incidentally, I am looking at your recent changes now. >> >> William >> Hello, >> >> >> There is one more thing to do, fix the documentation chapter numbering. >> Scilab should take the place of Tcl (39), do you confirm this ? >> >> Simon >> >> Le 19/08/2014 17:43, Simon Marchetto a écrit : >> > Hello, >> > >> > I did some minor modifications on braces for coding style, and a few >> > fixes in documentation. >> > I think this branch is ready and won't have any other changes in this >> > summer :-), except if you ask. >> > >> > Regards, >> > >> > Simon >> > >> > >> > Le 11/08/2014 20:02, William S Fulton a écrit : >> >> On 08/08/14 17:13, Simon Marchetto wrote: >> >>> Hello, >> >>> >> >>> I have fixed all the remaining issues. >> >>> For most of them, I follow your indications, for a few I could'nt. >> >>> Below you can find the details. >> >> Thanks, I'll look over them in the next few days. >> >>> I am having now a last look on the code and the documentation. >> >>> For example checking the coding style. About this : documentation says >> >>> to apply K&R style on C/C++ module source code. But on typemaps >> source >> >>> code ? >> >>> >> >> Use the same style as in the SWIG source code, but using spaces is >> >> better than tabs because you don't know what the tab setting for users >> >> is set to. Can you let me know if you're going to make changes in this >> >> area? >> >> >> >>> - Can you add the Swig banner to all generated files please, it is >> >>> missing in a few of them. >> >>> >> >>> >> >>> Which files ? There are files like loader.sce with no SWIG banner, >> >>> but they are generated by Scilab. >> >>> >> >> Okay, that makes sense then. >> >> >> >> William >> > >> > >> ------------------------------------------------------------------------------ >> > _______________________________________________ >> > Swig-devel mailing list >> > Swi...@li... >> > https://lists.sourceforge.net/lists/listinfo/swig-devel >> >> >> >> ------------------------------------------------------------------------------ >> Slashdot TV. >> Video for Nerds. Stuff that matters. >> http://tv.slashdot.org/ >> _______________________________________________ >> Swig-devel mailing list >> Swi...@li... >> https://lists.sourceforge.net/lists/listinfo/swig-devel >> > > |