← Back to team overview

kicad-developers team mailing list archive

Re: Rev 6350, abs vs std::abs

 

JP,

Please add <cstdlib> 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 <cstdlib>. 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 <http://ci.kicad-pcb.org/job/kicad-full/947/changes>
>>>
>>> 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 <http://ci.kicad-pcb.org/job/kicad-full/ws/>
>>> $ bzr revision-info -d <http://ci.kicad-pcb.org/job/kicad-full/ws/>
>>> info result: bzr revision-info -d <http://ci.kicad-pcb.org/job/kicad-full/ws/> returned 0. Command output: "6348 jp.charras@xxxxxxxxxx-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 <http://ci.kicad-pcb.org/job/kicad-full/ws/>
>>> info result: bzr revision-info -d <http://ci.kicad-pcb.org/job/kicad-full/ws/> returned 0. Command output: "6349 jp.charras@xxxxxxxxxx-20151207092431-90sr06sy0jhwwg2f
>>> " stderr: ""
>>> [kicad-full] $ bzr log -v -r revid:jp.charras@xxxxxxxxxx-20151207092209-7rby34qymt287ho6..revid:jp.charras@xxxxxxxxxx-20151207092431-90sr06sy0jhwwg2f --long --show-ids
>>> Getting local revision...
>>> $ bzr revision-info -d <http://ci.kicad-pcb.org/job/kicad-full/ws/>
>>> info result: bzr revision-info -d <http://ci.kicad-pcb.org/job/kicad-full/ws/> returned 0. Command output: "6349 jp.charras@xxxxxxxxxx-20151207092431-90sr06sy0jhwwg2f
>>> " stderr: ""
>>> RevisionState revno:6349 revid:jp.charras@xxxxxxxxxx-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: </usr/local>
>>> -- 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: <http://ci.kicad-pcb.org/job/kicad-full/ws/build>
>>> + 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
>>> -- Bazaar version control system version 2.1.1 found.
>>> [  2%] Built target lib_dxf
>>> [  2%] Built target idf3
>>> [  3%] Built target potrace
>>> [  3%] Built target pcb_calculator_lexer_source_files
>>> [  3%] Built target dxf2idf
>>> [ 32%] [ 32%] Built target bitmaps
>>> Built target idfcyl
>>> [ 32%] Built target lib-dependencies
>>> [ 32%] Built target idfrect
>>> [ 32%] Built target idf2vrml
>>> -- Writing <http://ci.kicad-pcb.org/job/kicad-full/ws/build/kicad_build_version.h> file with version: (2015-12-07 BZR 6349)
>>> [ 32%] Built target version_header
>>> Scanning dependencies of target polygon
>>> [ 32%] Building CXX object polygon/CMakeFiles/polygon.dir/math_for_graphics.cpp.o
>>> [ 34%] Built target gal
>>> [ 34%] Built target github_plugin
>>> <http://ci.kicad-pcb.org/job/kicad-full/ws/polygon/math_for_graphics.cpp>: In function ‘double GetPointToLineSegmentDistance(int, int, int, int, int, int)’:
>>> <http://ci.kicad-pcb.org/job/kicad-full/ws/polygon/math_for_graphics.cpp>:467: error: call of overloaded ‘abs(int)’ is ambiguous
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:94: note: candidates are: double std::abs(double)
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:98: note:                 float std::abs(float)
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:102: note:                 long double std::abs(long double)
>>> <http://ci.kicad-pcb.org/job/kicad-full/ws/polygon/math_for_graphics.cpp>:475: error: call of overloaded ‘abs(int)’ is ambiguous
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:94: note: candidates are: double std::abs(double)
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:98: note:                 float std::abs(float)
>>> /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/cmath:102: note:                 long double std::abs(long double)
>>> make[2]: *** [polygon/CMakeFiles/polygon.dir/math_for_graphics.cpp.o] Error 1
>>> make[1]: *** [polygon/CMakeFiles/polygon.dir/all] Error 2
>>> make[1]: *** Waiting for unfinished jobs....
>>> [ 37%] Built target pcbcommon
>>> Scanning dependencies of target common
>>> [ 37%] Building CXX object common/CMakeFiles/common.dir/build_version.cpp.o
>>> Linking CXX static library libcommon.a
>>> [ 44%] Built target common
>>> make: *** [all] Error 2
>>> Build step 'Execute shell' marked build as failure
>>> [WARNINGS] Skipping publisher since build result is FAILURE
>>
>>
>>
>> -- 
>> Jean-Pierre CHARRAS
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


References