← Back to team overview

kicad-developers team mailing list archive

Re: Rev 6350, abs vs std::abs

 

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


Follow ups

References