Code metrics

Reinhardt Behm wrote:
> On 10.03.2015 18:16, Stefan Reuther wrote:
>> Reinhardt Behm wrote:
>>> 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.

All required context is in the line of code and the warning: m_u16 is a
uint16_t structure member, and int has more than 16 bits. The only "fix"
to this warning is
    someptr->somearray[someindex].m_u16 =
              (uint16_t)(someptr->somearray[someindex].m_u16 + 1U);
and I doubt this makes the code more readable.

>>> 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.

DO-178 might be a better argument against schedule cuts and bad
requirements than SPICE, but somehow I doubt it...


  Stefan

On 3/11/2015 3:46 AM, Reinhardt Behm wrote:
> Don Y wrote:
>> On 3/10/2015 7:33 PM, Reinhardt Behm wrote:
>>> glen herrmannsfeldt wrote:
>>
>>>> Yes, if the code is right, I would rather go through the extra work
>>>> to explain it, that "fix" it.
>>>
>>> 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.
>>
>> +42
>>
>> The same holds true when walking backwards through time:  e.g., trying to
>> support old hardware with old tools.  You may not have a choice as to the
>> capabilities of the compiler, etc.
>
> That reminds me of compilers that where at least error free.
> For example I used C/80 for Z80 from '83 on until about 2000 intensively and
> did not find any error. And it did not warn much. ;-)
> Ok, the compiler was just 40k. So there was just no space for errors. ;-)

A lot of old compilers weren't very "clever".  I can recall projects in which
I was called on to "recover lost sources" where I could actually generate
a reasonable facsimile of the C sources from the binary executable (barring
object names, of course) -- because the code generator was so "predictable".

> In contrast to some compiler for another CPU I used around 2000. It was just
> a collection of bugs. We had to replace about 500 OTP chips because of a bug
> inserted by the compiler.
> It was from a company whose owner has several times in the past insisted in
> this group that open source is not reliable but his tools are thoroughly
> tested and verified.

Hmmm... I can think of at least three such denizens (though haven't seen posts
from any of them in quite a while)

>>> 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.
>>
>> But an explicit cast should have fixed that.
>
> func((uint8_t)1); looks nasty.

Would you prefer the rampant automatic type conversion of, e.g., Pascal?  :>

> True, but at that place the compiler could have deduced, that there is no
> loss by truncation.

Agreed.  But, I suspect the compiler was just looking at *types* and
not *values*.

On the one hand, we complain that the compilers are too smart -- offering
us "unwanted" advice.  On the other, not smart *enough*!  :-/

>> Would you complain if the compiler prodded you when you tried:
>>
>> float x;
>> x = sqrt(6);
>>
>> (two "warnings", there)
>

Reinhardt Behm <rbehm@hushmail.com> wrote:

(snip)

> That reminds me of compilers that where at least error free.
> For example I used C/80 for Z80 from '83 on until about 2000 intensively and 
> did not find any error. And it did not warn much. ;-)
> Ok, the compiler was just 40k. So there was just no space for errors. ;-)

The compilers I remember from the early 8086 days had an additional
pass for errors. Each pass was on one floppy, and you didn't need 
the final one if there weren't any errors.

-- glen

Hi Les,

On 3/11/2015 3:12 AM, Les Cargill wrote:
> Don Y wrote:
>> On 3/10/2015 4:35 PM, glen herrmannsfeldt wrote:
>>> Don Y <this@is.not.me.com> wrote:
>>>
>>>> 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.
>>
>> Well, it's not as if the compiler writer is trying to enforce
>> "coding guidelines"
>
> But they are. "There are rules" - Walter Sobchak.

There'a a difference between warning about a missing cast vs., e.g,
a SESE "violation".  The former could well indicate that you've done
something you didn't intend to (e.g., int from ptr) while the latter
is a stylistic issue.

Some tools now feel obliged to "improve your style" (e.g., MISRA)
to adhere to a particular set of "guidelines".

> Some of the more popular compilers just shaddup already about it unless the
> constraint violation is significant.

As I said upthread (wrt my *original* post), it's all about knowing how
to *use* metrics, guidelines, warnings, etc.

How many pieces of code (esp UI's) *force* the user to do things in a
fixed order -- UNNECESSARILY?  E.g., enter name; then address; then phone
number; etc. (poor example but meant to illustrate the total LACK of
need for any such ordering).

Years ago, I wrote a little DB "address book" application.  Fill in "City"
and (valid) choices for "State" are automatically presented.  Fill in State
and City choices appear.  Fill in Area Code and State choices are constrained.
etc.  Coding it wasn't significantly harder than REQUIRING the user to proceed
through the form in a *fixed* order (OK, now that I know what city he's in,
I can present him with state choices...).

[Developers, IME, have *a* vision of how a program will be used and often
IMPOSE that on their users.  Some of us no how to "get the hell out of the way"
and *assist* instead of *enforce*]

>> E.g., I *don't* like the UNIFORM practice of initializing
>> variables at their declaration (this isn't even possible in
>> all languages!).  Instead, I prefer to initialize them closer
>> to their first use.
>
> Oh, I think initialized variables are wonderful. But then again,
> I had to *implement* mutable initialized variables once - which is not
> nearly as cool as it sounds.

So, a *second* initialization is "OK", in your playbook -- when you
KNOW what the variable should ACTUALLY be initialized to?

> The toolchain simply located initialized variables to PROM, and uninitialized
> variables to RAM. So we made our own segments, then copied from initialized to
> a peer to BSS in the startup. There
> was more to it than just that, but that's the gist.
>
> It's funny - I see this an extension of the RAII principle, and then people go
> off on the exact history behind RAII in C++. Yes, but ...
>
>> It's tedious to have to scroll back to the top of a function to
>> discover where the variable was declared & initialized.  OTOH,
>> if you initialize it "when it becomes of interest", then you
>> are more likely to see that initialization in the "local" code.
>
> Modern 'C' means never having to do this. Declare a block for all the temp
> variables surrounding a thing:
>
> double tane = 0.0;
> double angle = 42.0;
> const double xepsi = ( 0.00000...1 ); // yadda yadda
> // at the end of this, tane is completed.
> {
>      const double sinX = sin(angle);
>          const double cosX = cos(angle);
>          tane = (abs(cosX)>xepsi) ? sinX/cosX : TOOBIG; // or whatever
>          ...
> }
>
> *NOT* doing this leads to all sort of pernicious headaches.

This only works when a variable's scope can be constrained to exactly
that block.  E.g., when you later realize you need "sinX" on the line
*after* (or, three stanzas later) the closing curly brace.

I am consistently bothered by attempts to make local declarations
that end up ADDING to my effort:
      limit: int
      ...
      limit = len foo;
      for (i:=0; i<limit; i++) {
         ...
         if (something)
            break;
         ...
      }
      if (i < limit)
         // loop aborted prematurely

(or, set a flag indicating "abort")

Then, when this fragment throws an error ("i out of scope"), having
to rewrite it:
      limit,i: int
      ...
      limit = len foo;
      for (i=0; i<limit; i++) {
         ...
         if (something)
            break;
         ...
      }
      if (i < limit)
         // loop aborted prematurely

This is particularly annoying for "disposable" variables (e.g., "i")
that may exist in *other* scopes.  E.g., imagine previous code fragment
embedded in another block THAT ALSO DEFINES an 'i'!  I.e., the final
conditional would NOT throw an error -- but would NOT perform the
test that you intended!

>> Gratuitously adding extra nested blocks just so you can declare/define
>> the variable more "locally" OUT OF HABIT clutters up the code.
>
> I vehemently, fundamentally and absolutely disagree. It organizes the code. The
> temps all go away at the bottom of the block. You will not get a sore wrist
> from a little scrolling. But having all references to
> an identifier be in the same general region *works*.
>
> For stuff that's globalish state there is 'find . -name "*.[ch]" | xargs grep
> -n ...'
>
> You need that anyway. No, your favorite IDE don't do that right
> either. :)
>
> Code tells a story. Sequentially organized little paragraphs are
> as natural as reading. Having to find the subroutine is harder than
> this.
>
> Of course, subroutines are the next logical step - if it's more than a few
> lines, the declarations are not the major point of it and/or
> it needs to be reused.

All blocks do is give you an way of inlining a *procedure* (not a *function*
because you can't pass a value *out* of a block).  I much prefer writing
procedures/functions with appropriate descriptive names and (possibly later)
opting to inline them.  This lets me see more "on a page".

Do you really need to see the temporary variables used in a memcpy(3c)
operation "inline" instead of a memcpy(3c) invocation?

>> And, for folks not accustomed to this, can confuse esp if you
>> reuse an identifier in this nested scope.
>
> It should take seconds to get used to.

And *hours* to chase down bugs like the overlapping scope issue, above.

>> The real problem lies in the squishy nature of C inherently being
>> at odds with the idea of "portability" (which requires CONSISTENCY
>> across platforms, etc.)
>
> Nah. The travails of this are Vastly overrated. People get too excited
> and overdo it. Again - the declarations are the key.

C works well if you are pedantic in your discipline.  I.e., no such thing
as an int, char, etc.  Everything explicitly qualified:  signedness, const,
static, etc.  And, no "cleverness" exploiting ASSUMED endianness or
specific encoding(s)!

Yet, you can still get screwed easily and frequently.  E.g., try doing
anything with struct packing, enums, bitfields, etc. in a way that is
even marginally portable.  sizeof() on these won't even give you consistent
results!  Or, exploiting the full range of values tolerated by particular
data types (try writing a bignum implementation or decimal arithmetic
package)

E.g., I have to *ensure* that the same compiler (or, one with EXACTLY the
same "implementation defined behaviors") is used to compile both the
client- and server-side stubs for my RPC's else what starts out as a "foo"
on one side can look like something else, entirely, on the *other* side!
And, that assumes a *homogeneous* environment (obvious problems when you
move to a heterogeneous environment)

[solution:  allow user to add cast operators that the IDL compiler
automatically invokes to provide more freedom of choice wrt tools]

Shopping day.  <frown>

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

(snip, I wrote)
>> 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.
 
> Well, it's not as if the compiler writer is trying to enforce
> "coding guidelines" -- that mechanism should be part of a different
> HIGHLY CONFIGURABLE tool.  Rather, it's (usually) trying to alert you
> of subtle behaviors of which many programmers may not be cognizant
> *or* particularly vigilant.

Yes, but eventually they run out of the true subtle ones and get
to the no-so-subtle ones. 

Or, some may be subtle in one context and not in another, but the
compiler doesn't know the difference.

(snip, I wrote) 
>> 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.
 
> The question becomes, "why couldn't you have come up with an
> appropriate initializer?"  Or, why is your program logic so
> "unpredictable" that the compiler can't sort this out with
> a static analysis?  (i.e., if the compiler can't sort it out,
> are you sure *people* will be much better at the task?)

The compilers are getting better.  I think most now figure out:

if(x==0) y=1; else y=2;

(not that you would do that, but something assigned in both
parts of an if/then/else.)  But they didn't always get that one.
Maybe they won't yet figure out nested if/then/else, though.

It doesn't happen all that often, though. 
 
> E.g., I *don't* like the UNIFORM practice of initializing
> variables at their declaration (this isn't even possible in
> all languages!).  Instead, I prefer to initialize them closer
> to their first use.
 
> It's tedious to have to scroll back to the top of a function to
> discover where the variable was declared & initialized.  OTOH,
> if you initialize it "when it becomes of interest", then you
> are more likely to see that initialization in the "local" code.

(snip)

-- glen

Hi Stefan,

On 3/10/2015 3:16 AM, Stefan Reuther wrote:
> Reinhardt Behm wrote:
>> On 10.03.2015 06:27, Stefan Reuther wrote:

>> 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")

Replace the increment operator with an explict add.

>> 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.)

Well, where do the obligations of the language/toolchain lie?
E.g., I can't imagine anything in a language or tool that will
compensate for a shitty boss!  :-)

> The more sustainable solution is educate people and develop best
> practices.

Of course!  And, this works well *if* you have control over those
people:  their "training" AND "selection"!

The problem arises when you have to deal with the INEVITABILITY
that some dweeb will come along after (or before) you and muck
things up.  Falling back on "well, it *works*, so..." as the
justification for it being in the state that it is.

"Sure, there are LOTS of warnings!  But, none of them (SEEM TO!)
matter to the operation of the code!"  And, by "operation" he
implies "correctness"!

    int multiply(int a, int b) { return a + b }

works fine for a=b=2!  :-/

> 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.

Various stake-holders are each trying to "improve" the quality of
our "product" (i.e., software).

Employers would like to be able to hire from a larger pool of
"adequately skilled" (as "adequately" approaches "infinity") -- and
at ever lower *payrates*.

Users would like to be able to interact with "accurate" (correct?)
products that behave in expected ways.

We would like to be "less hassled" by the *process* -- as well as
enabled to undertake ever more complex challenges.  Yet, without
loosing much "artistic license".

But, we're all stuck with tools that "come up short" -- in one way or
another.

E.g., the newer languages try to do much for you at the expense of
efficiency (time/space).  "Stricter" languages tend to force you to
do a certain thing in a certain way (which can be confining as well
as inefficient).  "Informal" languages (C) give us well-enough
rope to hang ourselves -- or, at least find our legs entangled in
it's loose coils!

It's a matter of trying to squeeze a balloon on *both* ends, simultaneously.

[BTW, I had a chance to play with the A5 dev board for a few minutes.
Seems like it will be a win for *my* application -- very "snappy".
Still have to round up the "optional components" and fully populate it,
though.  Cripes, what did they SAVE by omitting a few inexpensive parts??
Also have to see if I can mate a display to it, just for yucks.]

Hi Glen,

On 3/11/2015 10:24 AM, glen herrmannsfeldt wrote:
> Don Y <this@is.not.me.com> wrote:
>
> (snip, I wrote)
>>> 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.
>
>> Well, it's not as if the compiler writer is trying to enforce
>> "coding guidelines" -- that mechanism should be part of a different
>> HIGHLY CONFIGURABLE tool.  Rather, it's (usually) trying to alert you
>> of subtle behaviors of which many programmers may not be cognizant
>> *or* particularly vigilant.
>
> Yes, but eventually they run out of the true subtle ones and get
> to the no-so-subtle ones.

I see the goal as being the same:  "help" the developer catch things
before he has to embed that executable in something even more complex.

E.g., (effectively) complaining about forgetting a decimal point in
a float/double seems "petty":  "Why can't the compiler KNOW that there
should have been a decimal point, there?  It's obvious that the
value *must* be a float so interpret that integer constant *as*
a float, for crying-out-loud!"

I design my software (and hardware) to "strongly influence" my
successors to continue in the same style that I've adopted.  If
only for the sake of consistency!  I.e., I make it fairly easy for
you to *extend* my code along the same lines that I've adopted
instead of having to "build from scratch".  E.g., add an entry
to an existing const table instead of writing some ad hoc code, etc.

> Or, some may be subtle in one context and not in another, but the
> compiler doesn't know the difference.
>
> (snip, I wrote)
>>> 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.
>
>> The question becomes, "why couldn't you have come up with an
>> appropriate initializer?"  Or, why is your program logic so
>> "unpredictable" that the compiler can't sort this out with
>> a static analysis?  (i.e., if the compiler can't sort it out,
>> are you sure *people* will be much better at the task?)
>
> The compilers are getting better.  I think most now figure out:
>
> if(x==0) y=1; else y=2;
>
> (not that you would do that, but something assigned in both
> parts of an if/then/else.)  But they didn't always get that one.
> Maybe they won't yet figure out nested if/then/else, though.
>
> It doesn't happen all that often, though.

#define (FOO)  (...)
...
if (x == FOO) ...

What I need to explore is how clever tools are at optimizing:

const foo ...;
...
if (x == foo) ...

esp if foo is extern!  (I have been consistently moving away from
manifest constants in my code in favor of const variables)

On 15-03-11 18:36 , Don Y wrote:
> On 3/11/2015 3:46 AM, Reinhardt Behm wrote:
>> Don Y wrote:
>>> On 3/10/2015 7:33 PM, Reinhardt Behm wrote:
>>>> glen herrmannsfeldt wrote:
   [mucho snips]
>>>> 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.
>>>
>>> But an explicit cast should have fixed that.
>>
>> func((uint8_t)1); looks nasty.
>
> Would you prefer the rampant automatic type conversion of, e.g.,
> Pascal?  :>

I think Ada has the right solution here: a literal never has a type 
itself; it takes on the type expected in the context. So the literal 1 
means the mathematical, universal, integer "one", and is compatible with 
any other integer type (as long as the value 1 is included in the range 
of that type, of course).

-- 
Niklas Holsti
Tidorum Ltd
niklas holsti tidorum fi
       .      @       .

On 3/11/2015 10:56 AM, Niklas Holsti wrote:
>> Would you prefer the rampant automatic type conversion of, e.g.,
>> Pascal?  :>
>
> I think Ada has the right solution here: a literal never has a type itself; it
> takes on the type expected in the context. So the literal 1 means the
> mathematical, universal, integer "one", and is compatible with any other
> integer type (as long as the value 1 is included in the range of that type, of
> course).

I'm not sure even *that* "do the right thing" approach is 100% safe.

Consider:   x = sqrt(6)

Then, consider:  if (x == 6)

(in both cases, x being a floating type!)

In the latter case, I would want the compiler to force me to look
at what I wrote, AGAIN -- if only to add the DP (or a cast).  Hopefully,
it gets me thinking:
    An *exact* compare of a floating type??  Are you *really* sure you
    want to do that??

E.g., I can write:
    value := "1234" + 1
Do I intend that to yield a *string* having the value "12341"?
Or, an *in* having the value '1235'?
(Or, a string having the value "1235"??)

Forcing me to look at it again would lead me to make my intentions
more clear:
    value := (int) "1234" + 1

It's also too easy for people to "see what they want to see" when
writing code.  I suspect that accounts for most of those "hours
spent finding an OBVIOUS bug!"

What would you *expect* the values of each variable to be?
    x, y: int = 5;
vs.
    int x, y = 5;

Am 11.03.2015 um 11:20 schrieb Les Cargill:
> Hans-Bernhard Br�ker wrote:

>> That's no solution.  That's wishful thinking.

>> The basic problem is that C compilers are officially and explicitly
>> allowed to "warn" about whatever they damn well please, and they make
>> ample use of that leeway.  Trying to keep a lid on that can of worms is
>> an exercise in futility.  For all you know, tomorrow's compiler update
>> might add a "warning: no C code should be compiled on a Friday, 13th".

> I assume a Lebowski reference will work here: "There are rules" - Walter
> Sobchak.

It doesn't, because there are none.

> In fact, there is The 'C' Standard

So let's go look into the C Standard, and try to find any limitations on 
what the translator is allowed to emit a diagnostic message about.  And 
guess what we find? None!  There are only required messages, but _no_ 
forbidden ones.  Instead there's even a (non-normative, but 
enlightening) footnote explicitly allowing any and all additional 
warnings (C99 5.1.1.3, footnote 8).

>> So you face a choice: either you completely give up on re-using source
>> code verbatim from one project to the next (so: parallel maintenance of
>> multiple versions of your "code library", forever!), or you give up on
>> that strict "no warnings!" plan.

> Or just do the things that are necessary to make the warnings go away.

Nice plan, but again, as soon as you try to do that for more than one 
compiler with the same source code, this generally becomes impossible.

> This will involve understanding the promotion and constraint rules. If
> you don't keep those in mind, you *will* make *ugly* mistakes
> that will fail at the worst possible times.

That's assuming all warnings are _about_ promotions and constraint 
violations.  That assumption is generally far from correct.

>> This is not at all a question of capabilities (or lack thereof).  It's a
>> question of what do you do if, in the only available compiler for the
>> platform someone else has decided you'll be using, there's simply no
>> combination of compiler switches that still yields some truly necessary
>> warnings (e.g. "function called without a prototype declaration"),

> You really should have prototype declarations on general principle.

Which is why any warning level so low that it doesn't include 
missing-prototype warnings is, IMO, unacceptable.

>> but
>> keeps quiet about perfectly sane constructs (e.g.: "global pointer
>> initialized with address of a file-scope function/variable").

> That's because it cannot know any better. A "lint" tool that does some
> better probing might.

No, because that's not a question of probing.  No amount of probing, by 
any tool, can _deduce_ whether I made those object 'static' by mistake 
or by conscious decision.  The only way any tool can ever know that 
would be by me _telling_ it that I did it on purpose, right there in the 
source code, e.g. by adding a lint comment to that object's definition.