On 18/09/13 10:06, Tom Hacohen wrote: > On 18/09/13 06:51, Lucas De Marchi wrote: >> On Tue, Sep 17, 2013 at 6:21 AM, Tom Hacohen <tom.haco...@samsung.com> wrote: >>> On 17/09/13 08:30, Stefan Schmidt wrote: >>>> Hello. >>>> >>>> On 09/17/2013 07:44 AM, Chris Michael - Enlightenment Git wrote: >>>>> devilhorns pushed a commit to branch master. >>>>> >>>>> commit 64bc97c53c5c3772595f9d2321f9e19590d8a477 >>>>> Author: Chris Michael <cp.mich...@samsung.com> >>>>> Date: Mon Sep 16 11:40:30 2013 +0100 >>>>> >>>>> Remove __UNUSED__ from function declaration where parameter is >>>>> actually used. >>>> >>>> This brings an old topic back into my mind. >>>> >>>> Its not the first time we eagerly tagged parameters as unused because >>>> gcc warned about it and later started to use them without removing the >>>> unused label. This has the potential to screw us badly as it is up to >>>> the compiler to decide what to do with the parameter here. >>> >>> I don't know much about the exact implementation details of GCC, but I >>> find it very unlikely that GCC is allowed, or will ever actually do >>> anything about a *used* variable that is marked as unused. That just >>> sounds too crazy to be true. So I don't think we'll ever get screwed. >> >> It won't produce incorrect code. > > Yeah, that's what I said (and backed up with gcc docs after). > >> >>> >>>> >>>> Given how many callback and other signatures we have with user_data or >>>> other unused parameters we end up with 3630 EINA_UNUSED and even 71 >>>> __UNUSED__ in efl alone. All with the potential to be used at some point >>>> but forgotten to remove the label. >>> >>> Again, not really an issue. >>>> >>>> My proposal would be to use -Wno-unused-parameter in our CFLAGS to >>>> disable this warning and remove all EINA_UNUSED and __UNUSED__ from >>>> parameters. >>>> >>>> I know it has the downside that in the rare case where you add a >>>> parameter to a signature yourself (read: not using an existing function >>>> signature) you might add it and forgot to use it. Which will not >>>> reported as warning in this case. >>>> >>>> In my opinion the risk is higher than the benefit here. >>> >>> I disagree. I find this warning very useful when prototyping and >>> refactoring APIs (both internal and external). I would really hate >>> losing that in a mess of warnings. >> >> Then you turn it on once, for that part of the code. Having it as the >> default isn't helping, but instead requiring more work from others. > > 1. I'm not the only one who adds APIs and might accidentally add a > parameter that's not used. > 2. Reconfiguring just to change cflags (and back, because all the > warnings will hide the real warnings that I also care about while > developing) is torture and wasteful and will just mean people won't do it. > 3. Everyone should look at those when adding API, not just me. > > -- > Tom. >
I have to agree with Tom here. +1's for all 3 of his listed points. dh ------------------------------------------------------------------------------ LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99! 1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13. http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
