[Geany-devel] Ideas on increasing quality of plugins

[Lex Trotman]

On 13 March 2011 13:57, Colomban Wendling <lists.ban at herbesfolles.org> wrote:
> Le 13/03/2011 02:38, Lex Trotman a écrit :
>> On 13 March 2011 09:34, Colomban Wendling <lists.ban at herbesfolles.org> wrote:
>>> [...]
>>>  * -Warray-bounds: warns about some out-of-bound array subscripting
>>>  * -Wformat: warns about wrong format/arguments in printf-like functions
>>>  * -Wimplicit-int: warns if the return type of a function is not missing
>>
>> Actually warns when the function type IS missing, but know what you mean :-)
>
> Woops! I must have reworded my sentence incorrectly :D
>
>>>  [...]
>>>
>>> All of these but -Wpointer-arith, -Wshadow, -Wundef and -Wwrite-strings
>>> are part of GCC's -Wall.
>>> Maybe we might directly use -Wall, but it warns about some things that
>>> are not really needed, such as unused functions.
>>>
>>> Actually, apart -Wwrite-strings, these flags don't show so much warnings :)
>>>
>>> So (once again), do these flags seems reasonable to you?
>>
>> Well if they are warnings only, the more the merrier, I'd use -Wall
>> and -Wextras but noting that there is a possibility that some plugin
>> might need a customised list if there is a good reason why it is
>> correct and can't reasonably avoid some warning.  Up to the plugin dev
>> to tell us why, but hey we're reasonable aren't we? :-)
>
> The problem of -Wextra is that it adds -Wunused-parameter, which is a
> bit annoying when writing callbacks (you get tons of them, and you don't
> have to fix them).
> But it adds some cool stuff too (for example -Wmissing-parameter-type
> and -Wold-style-declaration). Maybe we would like to use -Wall -Wextra
> -Wno-unused-parameter, or only pick the warning we like from -Wextra.

You're right, I forgot about callbacks, -Wno-unused-parameter is
better than having to splash G_GNUC_UNUSED all through the code.

>
> I don't think we should add too much warnings with false-positive (like
> -Wunused-parameter) by default, because they are likely to:
>  * make the unexperienced programmer try to fix them, though actually
> they shouldn't be [1];
>  * "hide" some more interesting warnings between plenty of false ones;
>  * annoy some developers without good reason.

Yes, thats why I say some plugins might have to have customised lists
of options if they generate too many false positives, but I should
think most will be ok.
I guess customised option lists is a pain in the build scripts though,
your call.

>
>> Note most developers compile Geany itself with -Wall at least (iaw the
>> Hacking file) and Nick has some extras he likes.
>
> Of course -- I personally use the following for all my builds: :D
> -Wall -W -Wunused -Wunreachable-code -Wformat=2 -Wundef -Wshadow
> -Wpointer-arith -Wbad-function-cast -Wwrite-strings -Wconversion
> -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes
> -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute
> -Wredundant-decls -Wnested-externs -pedantic
> -Werror-implicit-function-declaration -fshow-column
>
> But these are maybe a little too much, they usually generate a bunch of
> warnings that should not be "fixed" since they are false-positive actually.

Yes, might be a *little* too much :-) Whilst they are all useful some
won't help prevent plugin crashes eg -Wunreachable-code

>
>> Also note that some warnings need at least -O2 so the compiler does
>> the dataflow tracing needed to detect the warning.
>
> Right, but -O2 also have some drawbacks when debugging, so I'm not sure
> we want to add it by default.

Yes, maybe only the release version should do O2 by default just to
ensure that the checks are done.

>
>> PS we are only talking about gcc flags here but I think we only supprt
>> gcc even on Windows, right Enrico?
>
> Actually I check whether the compiler understand each flag, so it would
> be easy to support any compiler. I only talk about GCC warnings because
> I only know GCC's flags, but if somebody knows some other, we might add
> them.

Ok.

Cheers
Lex

>
>
> Cheers,
> Colomban
>
>
> [1] for example, hiding an unused parameter warning by faking a usage
> (like "(void)param") generally only bloats the code without any benefit
>