[Swan-dev] why I added a consistency check to pbs_in_struct

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| The old behavior was that a NULL implied that the nested struct, if any,
| could be ignored.  I'd call that obvious behaviour.
| (passing a pbs when not needed was benign).

That's not what the contract said.  Ever.

| Now, if this is done wrong, and on a rarely executed code path, we've got a
| ticking time bomb.

Any code path not tested in the test suite is a ticking time bomb.
pbs_in_struct is the least of our worries in that regard.

We should have better coverage in our test suite.  But we don't.  (I
am the most delinquent in adding tests.)

The number one source of untested paths is likely error-handling code.
It's a lot of work to push Pluto into those states.  I don't expect
error-handling routines to use pbs_in_struct.

| It's no longer obvious from a simple visual check of the call that passing
| in NULL or not isn't going to explode.

That's the way code works.  If only all problems could be caught by a
simple visual check!

Silently abusing a contract is worse than being called on it.

It's best to move bug detection higher in this hierarchy:

- impossible to write code with the bug

- a compiler reports the bug

- static analysis reports the bug.

- the runtime detects the bug immediately

- the runtime detects the bug eventually

- the behaviour is obviously wrong

- the behaviour is wrong and undetected.

- the behaviour is wrong and only detected by your adversaries

It would have been better if I had created two different versions of
struct_desc (one for structs with variable parts and one for structs
without) and two different versions of pbs_in_struct.  Then the
compiler could detect violations as type violations.  But that would
have been a lot more code bashing.

| > I was prompted to add this because it would have caught a mistake we
| > made but never committed.  It has not caught any actual bugs in our
| > code.  The more checking, the more confidence we can have in our code.
| 
|  Would it have crashed pluto?

I wish.

If I remember correctly (no guarantee -- this was a transient state in
development a while back), the code got further before it went awry.
Then we had to backtrack to the cause.