Re: [Kicad-developers] Rev 6350, abs vs std::abs

2015-12-07 Thread Wayne Stambaugh 
JP,

Please add  to math_for_graphic.cpp to see if it resolves the
issue?  It does look a bit odd using both abs() and std::abs() in the
same source file.

Thanks,

Wayne

On 12/7/2015 8:28 AM, Chris Pavlina wrote:
> It is indeed difficult to test an issue that doesn't appear on your computer, 
> but from a quick look around, it looks like the correct way is to leave 
> std::abs, but add #include . Unlike stdlib.h, cstdlib defines 
> overloads for all the integer widths, reducing the possibility of a bad 
> mistake when using it with something longer than an int:
> 
> http://www.cplusplus.com/reference/cstdlib/abs/
> 
> Perhaps an oversight on my part not to have included that in the first place. 
> Maybe you could give that a try? The usage would be much more obvious then.
> 
> 
> On Mon, Dec 07, 2015 at 02:17:19PM +0100, jp charras wrote:
>> Le 07/12/2015 13:51, Chris Pavlina a écrit :
>>> Mostly directed at JP:
>>>
>>> Back in June, I went through and changed a few 'weird' things we were 
>>> doing, including using C abs instead of C++ std::abs:
>>>
>>> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/5831
>>>
>>> You just changed it back:
>>>
>>> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/6350
>>>
>>> What compiler could possibly have an issue with that? std::abs is, for very 
>>> good reason, the preferred way to find absolute value in C++. As such, if 
>>> there is some obscure reason we need to use abs(), would it kill you to 
>>> leave some explanation behind so nobody else also gets confused by that? A 
>>> real explanation in the commit message would be extremely helpful, as 
>>> possibly would a comment in the vicinity of the intentionally nonstandard 
>>> code.
>>>
>>> --
>>> Chris
>>
>>
>> std::abs is preferable to std, and I had no issue on my computer.
>> It is not so easy to fix an issue (using std::abs with integers,
>> std::abs used with floats did not created issues) which does not appears
>> on your computer.
>>
>> But here is my reason:
>>
>>> See 
>>>
>>> Changes:
>>>
>>> [jean-pierre charras] Kicad manager: fix a potential bug which could crash 
>>> Kicad manager. math_for_graphics.cpp: remove useless includes.
>>>
>>> --
>>> Started by an SCM change
>>> Building on master in workspace 
>>> $ bzr revision-info -d 
>>> info result: bzr revision-info -d 
>>>  returned 0. Command output: 
>>> "6348 jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6
>>> " stderr: ""
>>> [kicad-full] $ bzr pull --overwrite lp:kicad
>>> You have not informed bzr of your Launchpad ID, and you must do this to
>>> write to Launchpad or access private data.  See "bzr help launchpad-login".
>>> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product is 
>>> permanently redirected to 
>>> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/changes
>>> You have not informed bzr of your Launchpad ID, and you must do this to
>>> write to Launchpad or access private data.  See "bzr help launchpad-login".
>>>  M  kicad/class_treeproject_item.cpp
>>>  M  polygon/math_for_graphics.cpp
>>> All changes applied successfully.
>>> Now on revision 6349.
>>> [kicad-full] $ bzr revert
>>> $ bzr revision-info -d 
>>> info result: bzr revision-info -d 
>>>  returned 0. Command output: 
>>> "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
>>> " stderr: ""
>>> [kicad-full] $ bzr log -v -r 
>>> revid:jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6..revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
>>>  --long --show-ids
>>> Getting local revision...
>>> $ bzr revision-info -d 
>>> info result: bzr revision-info -d 
>>>  returned 0. Command output: 
>>> "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
>>> " stderr: ""
>>> RevisionState revno:6349 
>>> revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
>>> [kicad-full] $ /bin/sh -xe /tmp/hudson8903420887098700890.sh
>>> + OPTS=' -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
>>> -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
>>> -DKICAD_SCRIPTING_WXPYTHON=ON'
>>> + '[' -d build ']'
>>> + cd build
>>> + /usr/bin/cmake .. -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
>>> -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
>>> -DKICAD_SCRIPTING_WXPYTHON=ON
>>> -- Kicad install dir: 
>>> -- Check for installed OpenGL -- found
>>> -- Found Glew: /usr/lib64/libGLEW.so
>>> -- Check for installed GLEW -- found
>>> -- Check for installed Cairo -- found
>>> -- Check for installed Python Interpreter -- found
>>> -- Python module install path:

Re: [Kicad-developers] Rev 6350, abs vs std::abs

2015-12-07 Thread jp charras 
Le 07/12/2015 14:28, Chris Pavlina a écrit :
> It is indeed difficult to test an issue that doesn't appear on your
> computer, but from a quick look around, it looks like the correct way
> is to leave std::abs, but add #include . Unlike stdlib.h,
> cstdlib defines overloads for all the integer widths, reducing the
> possibility of a bad mistake when using it with something longer than
> an int:
> 
> http://www.cplusplus.com/reference/cstdlib/abs/
> 
> Perhaps an oversight on my part not to have included that in the
> first place. Maybe you could give that a try? The usage would be much
> more obvious then.


Looks like including both cstdlib (for ints) and cmath (for floats) is
needed for std::abs.

Fixed now.

Thanks.


-- 
Jean-Pierre CHARRAS

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

Re: [Kicad-developers] Rev 6350, abs vs std::abs

2015-12-07 Thread Chris Pavlina 
It is indeed difficult to test an issue that doesn't appear on your computer, 
but from a quick look around, it looks like the correct way is to leave 
std::abs, but add #include . Unlike stdlib.h, cstdlib defines 
overloads for all the integer widths, reducing the possibility of a bad mistake 
when using it with something longer than an int:

http://www.cplusplus.com/reference/cstdlib/abs/

Perhaps an oversight on my part not to have included that in the first place. 
Maybe you could give that a try? The usage would be much more obvious then.


On Mon, Dec 07, 2015 at 02:17:19PM +0100, jp charras wrote:
> Le 07/12/2015 13:51, Chris Pavlina a écrit :
> > Mostly directed at JP:
> > 
> > Back in June, I went through and changed a few 'weird' things we were 
> > doing, including using C abs instead of C++ std::abs:
> > 
> > http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/5831
> > 
> > You just changed it back:
> > 
> > http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/6350
> > 
> > What compiler could possibly have an issue with that? std::abs is, for very 
> > good reason, the preferred way to find absolute value in C++. As such, if 
> > there is some obscure reason we need to use abs(), would it kill you to 
> > leave some explanation behind so nobody else also gets confused by that? A 
> > real explanation in the commit message would be extremely helpful, as 
> > possibly would a comment in the vicinity of the intentionally nonstandard 
> > code.
> > 
> > --
> > Chris
> 
> 
> std::abs is preferable to std, and I had no issue on my computer.
> It is not so easy to fix an issue (using std::abs with integers,
> std::abs used with floats did not created issues) which does not appears
> on your computer.
> 
> But here is my reason:
> 
> > See 
> > 
> > Changes:
> > 
> > [jean-pierre charras] Kicad manager: fix a potential bug which could crash 
> > Kicad manager. math_for_graphics.cpp: remove useless includes.
> > 
> > --
> > Started by an SCM change
> > Building on master in workspace 
> > $ bzr revision-info -d 
> > info result: bzr revision-info -d 
> >  returned 0. Command output: 
> > "6348 jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6
> > " stderr: ""
> > [kicad-full] $ bzr pull --overwrite lp:kicad
> > You have not informed bzr of your Launchpad ID, and you must do this to
> > write to Launchpad or access private data.  See "bzr help launchpad-login".
> > http://bazaar.launchpad.net/~kicad-product-committers/kicad/product is 
> > permanently redirected to 
> > http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/changes
> > You have not informed bzr of your Launchpad ID, and you must do this to
> > write to Launchpad or access private data.  See "bzr help launchpad-login".
> >  M  kicad/class_treeproject_item.cpp
> >  M  polygon/math_for_graphics.cpp
> > All changes applied successfully.
> > Now on revision 6349.
> > [kicad-full] $ bzr revert
> > $ bzr revision-info -d 
> > info result: bzr revision-info -d 
> >  returned 0. Command output: 
> > "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> > " stderr: ""
> > [kicad-full] $ bzr log -v -r 
> > revid:jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6..revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> >  --long --show-ids
> > Getting local revision...
> > $ bzr revision-info -d 
> > info result: bzr revision-info -d 
> >  returned 0. Command output: 
> > "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> > " stderr: ""
> > RevisionState revno:6349 
> > revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> > [kicad-full] $ /bin/sh -xe /tmp/hudson8903420887098700890.sh
> > + OPTS=' -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
> > -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
> > -DKICAD_SCRIPTING_WXPYTHON=ON'
> > + '[' -d build ']'
> > + cd build
> > + /usr/bin/cmake .. -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
> > -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
> > -DKICAD_SCRIPTING_WXPYTHON=ON
> > -- Kicad install dir: 
> > -- Check for installed OpenGL -- found
> > -- Found Glew: /usr/lib64/libGLEW.so
> > -- Check for installed GLEW -- found
> > -- Check for installed Cairo -- found
> > -- Check for installed Python Interpreter -- found
> > -- Python module install path: lib/python2.6/site-packages
> > -- wxPython version 3.0 found.
> > -- Configuring done
> > -- Generating done
> > -- Build files have been written to: 
> > 
> > + rm -f

[Kicad-developers] Rev 6350, abs vs std::abs

2015-12-07 Thread Chris Pavlina 
Mostly directed at JP:

Back in June, I went through and changed a few 'weird' things we were doing, 
including using C abs instead of C++ std::abs:

http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/5831

You just changed it back:

http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/6350

What compiler could possibly have an issue with that? std::abs is, for very 
good reason, the preferred way to find absolute value in C++. As such, if there 
is some obscure reason we need to use abs(), would it kill you to leave some 
explanation behind so nobody else also gets confused by that? A real 
explanation in the commit message would be extremely helpful, as possibly would 
a comment in the vicinity of the intentionally nonstandard code.

--
Chris


___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

Re: [Kicad-developers] Rev 6350, abs vs std::abs

2015-12-07 Thread jp charras 
Le 07/12/2015 13:51, Chris Pavlina a écrit :
> Mostly directed at JP:
> 
> Back in June, I went through and changed a few 'weird' things we were doing, 
> including using C abs instead of C++ std::abs:
> 
> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/5831
> 
> You just changed it back:
> 
> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/6350
> 
> What compiler could possibly have an issue with that? std::abs is, for very 
> good reason, the preferred way to find absolute value in C++. As such, if 
> there is some obscure reason we need to use abs(), would it kill you to leave 
> some explanation behind so nobody else also gets confused by that? A real 
> explanation in the commit message would be extremely helpful, as possibly 
> would a comment in the vicinity of the intentionally nonstandard code.
> 
> --
> Chris


std::abs is preferable to std, and I had no issue on my computer.
It is not so easy to fix an issue (using std::abs with integers,
std::abs used with floats did not created issues) which does not appears
on your computer.

But here is my reason:

> See 
> 
> Changes:
> 
> [jean-pierre charras] Kicad manager: fix a potential bug which could crash 
> Kicad manager. math_for_graphics.cpp: remove useless includes.
> 
> --
> Started by an SCM change
> Building on master in workspace 
> $ bzr revision-info -d 
> info result: bzr revision-info -d 
>  returned 0. Command output: 
> "6348 jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6
> " stderr: ""
> [kicad-full] $ bzr pull --overwrite lp:kicad
> You have not informed bzr of your Launchpad ID, and you must do this to
> write to Launchpad or access private data.  See "bzr help launchpad-login".
> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product is 
> permanently redirected to 
> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/changes
> You have not informed bzr of your Launchpad ID, and you must do this to
> write to Launchpad or access private data.  See "bzr help launchpad-login".
>  M  kicad/class_treeproject_item.cpp
>  M  polygon/math_for_graphics.cpp
> All changes applied successfully.
> Now on revision 6349.
> [kicad-full] $ bzr revert
> $ bzr revision-info -d 
> info result: bzr revision-info -d 
>  returned 0. Command output: 
> "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> " stderr: ""
> [kicad-full] $ bzr log -v -r 
> revid:jp.char...@wanadoo.fr-20151207092209-7rby34qymt287ho6..revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
>  --long --show-ids
> Getting local revision...
> $ bzr revision-info -d 
> info result: bzr revision-info -d 
>  returned 0. Command output: 
> "6349 jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> " stderr: ""
> RevisionState revno:6349 
> revid:jp.char...@wanadoo.fr-20151207092431-90sr06sy0jhwwg2f
> [kicad-full] $ /bin/sh -xe /tmp/hudson8903420887098700890.sh
> + OPTS=' -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
> -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
> -DKICAD_SCRIPTING_WXPYTHON=ON'
> + '[' -d build ']'
> + cd build
> + /usr/bin/cmake .. -DCMAKE_BUILD_TYPE=Debug -DBUILD_GITHUB_PLUGIN=ON 
> -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON 
> -DKICAD_SCRIPTING_WXPYTHON=ON
> -- Kicad install dir: 
> -- Check for installed OpenGL -- found
> -- Found Glew: /usr/lib64/libGLEW.so
> -- Check for installed GLEW -- found
> -- Check for installed Cairo -- found
> -- Check for installed Python Interpreter -- found
> -- Python module install path: lib/python2.6/site-packages
> -- wxPython version 3.0 found.
> -- Configuring done
> -- Generating done
> -- Build files have been written to: 
> 
> + rm -f pcbnew/scripting/pcbnewPYTHON_wrap.cxx.o
> + rm -f pcbnew/scripting/pcbnewPYTHON_wrap.cxx
> + make -j4
> [  0%] Built target page_layout_lexer_source_files
> [  1%] Built target boost
> [  1%] Generating version string header
> [  1%] Built target netlist_lexer_source_files
> -- Using Bazaar to determine build version string.
> [  1%] Built target fp_lib_table_lexer_source_files
> [  1%] Generating headers containing GLSL source code
> [  1%] Built target pcb_lexer_source_files
> Headers are up-to-date
> [  1%] Built target shader_headers
> [  1%] [  1%] Built target specctra_lexer_source_files
> Built target pcb_plot_lexer_source_files
> [  1%] Built target cmp_library_lexer_source_files
> [  1%] Built target avhttp
> [  1%] Built target dialog_bom_cfg_lexer_source_files
> [  1%] Built target field_template_lexer_source_files