Code metrics

Stefan Reuther wrote:

> Some static checking tools (Klocwork, Coverity?) have a database and
> you can set their warnings to "ignore" there.

By line?  Or by kind?

> However, such a tool is too clunky for a edit/compile/test cycle for
> my taste. And not seeing the forest for the trees in compile output
> doesn't help too much either, even if the compiler warnings are
> declared nonexistant by a later step.

It should definitely be a tool which fits into the edit/compile cycle.

Maybe it should be inserted as a filter on the compiler output?

Greetings,

Jacob
-- 
"I am an old man now, and when I die and go to Heaven there are two matters
 on which I hope enlightenment. One is quantum electro-dynamics and the
 other is turbulence of fluids. About the former, I am rather optimistic."
 Sir Horace Lamb.

Jacob Sparre Andersen wrote:
> Stefan Reuther wrote:
>>Some static checking tools (Klocwork, Coverity?) have a database and
>>you can set their warnings to "ignore" there.
> 
> By line?  Or by kind?

By warning instance. They claim to be able to track issues even if the
source changes, so if you get a new warning of the same kind at a
different place, that's a new one, but if the current one moves, that'll
stay the same one.


  Stefan

On Tue, 10 Mar 2015 10:22:09 +0100, Jacob Sparre Andersen
<jacob@jacob-sparre.dk> wrote:

>Stefan Reuther wrote:
>
>> Some static checking tools (Klocwork, Coverity?) have a database and
>> you can set their warnings to "ignore" there.
>
>By line?  Or by kind?
>
>> However, such a tool is too clunky for a edit/compile/test cycle for
>> my taste. And not seeing the forest for the trees in compile output
>> doesn't help too much either, even if the compiler warnings are
>> declared nonexistant by a later step.
>
>It should definitely be a tool which fits into the edit/compile cycle.
>
>Maybe it should be inserted as a filter on the compiler output?


Dealing with that for a single compiler is not too bad, most let you
insert a pragma to disable a warning.  Just insert the disable before
the line of code in question, and reenable after, and you're good.

Then you get to the second compiler, and ugh...

The worst is when you get a compiler that starts warning you about
something perfectly legit, or even a standard idiom (MSVC likes to
warn on the standard idiom where you wrap the code in a macro with "do
{...} while(0)", for example).

We have a no-warnings rule here, along with high warning levels, and
it can certainly be a game of whack-a-mole at times.

At least something like PC-Lint lets you use the same annotations on
multiple platforms.

Reinhardt Behm wrote:
> On 10.03.2015 06:27, Stefan Reuther wrote:
>> Jacob Sparre Andersen wrote:
>>> Don Y wrote:
>>>> E.g., I target "no warnings" in my compiles.  But, that's because there
>>>> isn't a language feature that allows me to insert:
>>>>    #acknowledge  The 'missing cast' error can be ignored, here
>>>
>>> I have considered keeping a log of the expected warnings (with
>>> documentation of why the warnings are expected) and then making it an
>>> error to have a different set of warnings than exactly the documented
>>> set.
>>
>> Some static checking tools (Klocwork, Coverity?) have a database and you
>> can set their warnings to "ignore" there.
>>
>> However, such a tool is too clunky for a edit/compile/test cycle for my
>> taste. And not seeing the forest for the trees in compile output doesn't
>> help too much either, even if the compiler warnings are declared
>> nonexistant by a later step.
> 
> I have a simple rule: There may be not warnings.

That's where this subthread started.

> Simple things like a warning about a missing cast will be fixed.

Then, please fix:
      someptr->somearray[someindex].m_u16++;
("warning: conversion from int to uint16_t may alter its value")

> Others in my experience are most often the result of bad programming
> style and _have_ to be fixed. Possibly by not removing the warning but
> the programmer.

This sounds great in theory, but is not such a good idea if your project
is behind schedule and HR is already bringing on people from across the
continent. (Plus, workers' rights legislation probably does not allow
"you programmed a warning" as a firing reason.)

The more sustainable solution is educate people and develop best
practices. Things like: make your Thread::run function return void, not
int, or void* (POSIX), or DWORD (Win32). This avoids warnings ("no
return statement") in daemon threads and also avoids that people try to
take the shortcut and return fancy things through it. Or: do not put the
"myenum_MAX" value at the end of enums; instead, make it a separate
'static const uint'. This avoids the "enum not handled" warning and also
the mixed-type comparison warnings mandated by MISRA when you're
checking a user-supplied integer whether it fits into the enum. But that
needs time (and resources) to develop, and permission to refactor.


  Stefan

Robert Wessel wrote:

> Dealing with that for a single compiler is not too bad, most let you
> insert a pragma to disable a warning.  Just insert the disable before
> the line of code in question, and reenable after, and you're good.

No.  That doesn't tell which warning you've disabled checking for (at
least not with the compilers and tools I have experience with).  And it
doesn't tell you if the compiler/tool doesn't find a reason for a
warning there.

> Then you get to the second compiler, and ugh...

Yes. :-(

So far I usually don't have the pleasure of developing projects for
compilation with multiple compilers, but it is definitely something that
may make life "interesting" - even if it technically shouldn't be much
worse than handling warnings from various tools (besides the compiler).

Greetings,

Jacob
-- 
"But I mean it's so unfair ... but I don't know who for yet" - Djibard

On 10.03.2015 18:16, Stefan Reuther wrote:
> Reinhardt Behm wrote:
>> On 10.03.2015 06:27, Stefan Reuther wrote:
>>> Jacob Sparre Andersen wrote:
>>>> Don Y wrote:
>>>>> E.g., I target "no warnings" in my compiles.  But, that's because there
>>>>> isn't a language feature that allows me to insert:
>>>>>     #acknowledge  The 'missing cast' error can be ignored, here
>>>>
>>>> I have considered keeping a log of the expected warnings (with
>>>> documentation of why the warnings are expected) and then making it an
>>>> error to have a different set of warnings than exactly the documented
>>>> set.
>>>
>>> Some static checking tools (Klocwork, Coverity?) have a database and you
>>> can set their warnings to "ignore" there.
>>>
>>> However, such a tool is too clunky for a edit/compile/test cycle for my
>>> taste. And not seeing the forest for the trees in compile output doesn't
>>> help too much either, even if the compiler warnings are declared
>>> nonexistant by a later step.
>>
>> I have a simple rule: There may be not warnings.
>
> That's where this subthread started.
>
>> Simple things like a warning about a missing cast will be fixed.
>
> Then, please fix:
>        someptr->somearray[someindex].m_u16++;
> ("warning: conversion from int to uint16_t may alter its value")
>
I can't without more context.

>> Others in my experience are most often the result of bad programming
>> style and _have_ to be fixed. Possibly by not removing the warning but
>> the programmer.
>
> This sounds great in theory, but is not such a good idea if your project
> is behind schedule and HR is already bringing on people from across the
> continent. (Plus, workers' rights legislation probably does not allow
> "you programmed a warning" as a firing reason.)

But I have to get through DO-178 certification. I does not help to just 
crank out code, even if it would work.
I don't need to fire him/her, but that person should not be part of the 
project team before some real education. I agree with your next 
paragraph, of course.
And with proper planning - and that _is_ part of the software 
development process - the sudden need for more manpower should not arise.

>
> The more sustainable solution is educate people and develop best
> practices. Things like: make your Thread::run function return void, not
> int, or void* (POSIX), or DWORD (Win32). This avoids warnings ("no
> return statement") in daemon threads and also avoids that people try to
> take the shortcut and return fancy things through it. Or: do not put the
> "myenum_MAX" value at the end of enums; instead, make it a separate
> 'static const uint'. This avoids the "enum not handled" warning and also
> the mixed-type comparison warnings mandated by MISRA when you're
> checking a user-supplied integer whether it fits into the enum. But that
> needs time (and resources) to develop, and permission to refactor.
>
Totally agreed.
>
>    Stefan
>


-- 
Reinhardt

Hi Jacob,

On 3/10/2015 2:17 AM, Jacob Sparre Andersen wrote:
> Don Y wrote:

>> In a GUI IDE, one could conceivably tag (click) each warning's
>> corresponding source and have the IDE remember "this warning is OK,
>> here".  But, unless you can encode that in the source itself, its not
>> portable to other tools.
>
> Yes.  But it it is possible to encode it in the source.  The problem is
> to do it robustly - and in a way that doesn't annoy the programmer.

I think both of those criteria may prove to be elusive!
You can't count on pragmas (portably).

You *could* use comments -- but how do you tie a particular comment
to a particular line of code?  What if a line throws multiple warnings?
Esp if you want to explain why a warning is unnecessary!  (if you're
going to go to that length, why not just fix the code??)

You could wrap the offending term/expression/etc in a macro that
doesn't alter the code generated -- but, isn't that the same effort
as fixing the code?

Etc.

>>> Keeping a log of expected warnings and checking compilation and tool
>>> results against that list is of course a solution, but it feels too
>>> much like a hack.
>>
>> Agreed.  And, too "manual"/disciplined.
>
> What I've considered is to keep the "log" of expected warnings as
> specially formatted comments in the source, and then write a tool which
> correlates the compiler and tool output with the expected warning
> markers in the source files.

See above.  At what point does the programmer decide its just easier to
silence the warning (perhaps INCORRECTLY doing so)?

> It is still an extra step to do, but it would be easy to integrate it in
> my existing build and test framework, once the tool was written.

You'd also face problems when the compiler was revised (or, in my
case, when you move to a different compiler).  You'd have to create a set of
"named exceptions" and then find a way to map specific warnings from
specific compilers *to* those exceptions.  (this would also give you
a way to handle compilers that don't warn about as many things)

>> I believe people are inherently lazy.  If you *require* them to
>> perform some action, there is a good chance that they will
>> (eventually) fail to do so.  E.g., remove the comment alerting the
>> developer to the warning (hence, make that a hard error).
>>
>> OTOH, taking the approach of "treat all warnings as errors" (compiler
>> flag) ends up getting folks to just insert <whatever> to placate the
>> compiler.  Without considering the nature of the warning and *why*
>> (if?) their action should compensate, logically.
>
> Exactly.  But what is the solution then?  To accept warnings as
> *warnings* until the reason can be peer-reviewed?

No.  *My* solution is to just not accept warnings.  It means that
each time I port code to a different platform, toolchain, etc. I
end up having to tweek the code a bit.  Sometimes, compilers catch
behaviors that I was sloppy with (because older compilers gave me
more "rope" -- with which to hang myself!).  Other times, the
compiler is just "awfully helpful".  <shrug>

The *language* needs to be fixed instead of inventing kludges to
bolt on capabilities that the compiler should have inherently.

Personally, I'd love much stricter type checking, etc.

E.g., if foo() can return three values {A, B, C}, then I'd rather have
a type called foo_return_type_t that is an enumeration of those
three values (regardless of their *actual* "values") so statements
like:
     if (foo(...) < 2)  {}
get flagged.  "What the hell is '2'"?  -- even if, e.g., (effectively)
     #define (B) (2)
"Oh, you mean 'if (foo(...) < B)'?"

Don Y <this@is.not.me.com> wrote:

(snip on warnings, compilers, and debugging)

>> Yes.  But it it is possible to encode it in the source.  The problem is
>> to do it robustly - and in a way that doesn't annoy the programmer.
 
> I think both of those criteria may prove to be elusive!
> You can't count on pragmas (portably).
 
> You *could* use comments -- but how do you tie a particular comment
> to a particular line of code?  What if a line throws multiple warnings?
> Esp if you want to explain why a warning is unnecessary!  (if you're
> going to go to that length, why not just fix the code??)

But what it the code isn't broken?  Just because some compiler
writer thought that you shouldn't do something doesn't mean that
it is wrong. 

Note that Java requires the compiler to figure out that an assignment
is made to a scalar variable (not arrays, though) before the value is
used. Compilers are getting better, but there are still some cases
that the compiler can't figure out.  I then put an initializer on
it with a comment like 

int i=0;   /* the compiler didn't figure this out!!! */

Yes, if the code is right, I would rather go through the extra work
to explain it, that "fix" it.

-- glen

On Tue, 10 Mar 2015 11:46:51 +0100, Jacob Sparre Andersen
<jacob@jacob-sparre.dk> wrote:

>Robert Wessel wrote:
>
>> Dealing with that for a single compiler is not too bad, most let you
>> insert a pragma to disable a warning.  Just insert the disable before
>> the line of code in question, and reenable after, and you're good.
>
>No.  That doesn't tell which warning you've disabled checking for (at
>least not with the compilers and tools I have experience with).  And it
>doesn't tell you if the compiler/tool doesn't find a reason for a
>warning there.


In MSVC you'd do something ilke:

#pragma warning(disable : 4701)
  ...some code causing the warning
#pragma warning(default : 4701)

Or a tiny bit better:

#pragma warning(push)
#pragma warning(disable : 4701)
  ...some code causing the warning
#pragma warning(pop)


In GCC it's similar:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuninitialized"
  ...some code causing the warning
#pragma GCC diagnostic pop

glen herrmannsfeldt wrote:

> Don Y <this@is.not.me.com> wrote:
> 
> (snip on warnings, compilers, and debugging)
> 
>>> Yes.  But it it is possible to encode it in the source.  The problem is
>>> to do it robustly - and in a way that doesn't annoy the programmer.
>  
>> I think both of those criteria may prove to be elusive!
>> You can't count on pragmas (portably).
>  
>> You *could* use comments -- but how do you tie a particular comment
>> to a particular line of code?  What if a line throws multiple warnings?
>> Esp if you want to explain why a warning is unnecessary!  (if you're
>> going to go to that length, why not just fix the code??)
> 
> But what it the code isn't broken?  Just because some compiler
> writer thought that you shouldn't do something doesn't mean that
> it is wrong.
> 
> Note that Java requires the compiler to figure out that an assignment
> is made to a scalar variable (not arrays, though) before the value is
> used. Compilers are getting better, but there are still some cases
> that the compiler can't figure out.  I then put an initializer on
> it with a comment like
> 
> int i=0;   /* the compiler didn't figure this out!!! */
> 
> Yes, if the code is right, I would rather go through the extra work
> to explain it, that "fix" it.
> 
> -- glen

That is one reason why I am also very picky when choosing new CPUs. If there 
are only adequate tools like compilers I might not use the newest fancy CPU.
It is just too much effort.

For example I found a compiler from a CPU vendor which complained about a 
function defined with an uint8_t parameter and being called with a constant 
1. It saw the constant as an int and warned about an int being truncated to 
a uint8_t. After more of such nonsense and no other compiler available the 
CPU was out.

-- 
Reinhardt