Hi, On 21 December 2016 at 08:06, Cedric BAIL <cedric.b...@free.fr> wrote:
> On Mon, Dec 19, 2016 at 11:12 PM, Jean-Philippe André <j...@videolan.org> > wrote: > > Hey Mike, > > > > On 20 December 2016 at 01:12, Mike Blumenkrantz < > > michael.blumenkra...@gmail.com> wrote: > >> discomfitor pushed a commit to branch master. > >> > >> http://git.enlightenment.org/core/efl.git/commit/?id= > >> 25792d64165ad4f5f647a36f087af2d2206a6618 > >> > >> commit 25792d64165ad4f5f647a36f087af2d2206a6618 > >> Author: Mike Blumenkrantz <zm...@osg.samsung.com> > >> Date: Mon Dec 19 11:08:43 2016 -0500 > >> > >> build: enable -Wfloat-equal for compiling > >> > >> #WarningOfTheMonth > >> > > > > Urgh. What the hell. > > > > Those warnings have a very limited value. How many real issues have you > > seen that resulted from this unsafe comparison? I'm pretty sure that > float > > == 0 or float != 0 will succeed when we expect them to, and the equality > > tests will also work whenever we do direct assignments (without > arithmetic). > > I have found some real issue already in edje_calc, edje_load and > edje_edit (I believe some of the logic in eina matrix and quaternion > where also buggy in that regard, but less sure of it as I don't know > the user of the API that well). Overall, I wouldn't have even looked > for a problem there and did react at first like you when I saw those > warning. Still I realize that there was potential for edje and > ephysics to actually have bugs due to this that most people wouldn't > even know where to start looking at. Basically there is benefit for > this flag, but mostly in place where we are barelly looking. > You have seen potential issues from reading the code. I'm not arguing there aren't any real issues behind this, but I doubt there even is a ticket related to this on Phab. > Now I understand why cedric made a series of patches with the float > > comparison thing. I'm sure he didn't have anything more important to do. > > Are you going to fix the remaining 523 warnings? Please don't send 100 > > patches for that... > > We really should. I have no idea at this point which one are real > potential bug and which one aren't. Now why I am doing it per files > and not as a huge batch, because if we do get it wrong for one widget > or one feature it is easier to just revert that file that the entire > patch. I am not sure it is a good idea to do it in one go. > Urgh. After that you can go and fix every typo in the doc, one patch at a time. > PS: I had only 2 warnings before that. Potentially not fixable (va_start > > with char arguments in EAPI functions). But at least I could easily spot > > anything new. > > I am hoping to get it to a manageable level today and may switch to > more important work after that. Hopefully some more people can look at > it and fix the part that they feel confortable with. > I may have not expressed myself clearly then. My point really was exactly what bu5m4n said. If you want to add this kind of compiler warning, fix the warnings first, maybe even talk about it on the ML so it becomes a team effort. But don't pollute everyone with 500+ warnings that then hide new real issues on stuff we are working on. Should we add -Wfloat-conversion next? #WarningOfTheYear -- Jean-Philippe André ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/intel _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
