On Sat, Mar 11, 2017 at 03:41:53PM +0200, Lubomir I. Ivanov wrote: > On 11 March 2017 at 07:21, Jérémie Guichard <djebr...@gmail.com> wrote: > > Hey guys, > > > > I've added the flag conditionally as discussed in previous mail, and updated > > the pull request. > > i've tested the changes locally on Windows and it builds fine. > llrint() is also available. > > 4.9.0 seems to be the version they've added "float-conversation", so > this is correct: > > + if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.9.0") > + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wfloat-conversion") > + endif() > > of course, there is no need of lrint() in a lot of locations and > rint() can be used instead, like here: > > - sample->depth.mm = depth * FEET * 1000; > + sample->depth.mm = lrint(depth * FEET * 1000);
I find changes like this not only unnecessary but also potentially confusing. > here, t1 is of type int as well: > + int t1 = lrint(max_d / slope); > but that's not a big issue as long as numbers are not truncated for > 64bit builds. > our main Windows build is 32bit, so this is pretty much in effect: > sizeof(long) == sizeof(int). I keep wondering at which point it would make sense to switch to 64bit builds for Windows as well. Our stats still show a small but fairly stable share of 32bit systems running Subsurface. But then again, we also have a few people on 4.5 running Windows XP :-) > i haven't tested if the changes break functionality in some way, hopefully > not. If we had better tests, we could use those to test that nothing breaks... /D _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface