Re: Warning elimination patches in cp/call.c

[Craig Burley <burley at gnu dot org>]

See gcc/f/com.c ffecom_expr_intrinsic_, the huge long switch statement,
for an example of the usefulness of avoiding `default:' in some
situations.

Here, the statement evolved to its current form for a variety of
reasons, but the main reason to avoid `default:' now is to ask
for gcc's help identifying intrinsics that have been added to the
table of intrinsics (gcc/f/intrin.def) but for which no actual
code-generation support has been added.  That avoids mistakes I've
frequently made in the past, in released versions of g77.

The cost is a "huge" number of unnecessary `case' statements.  Note
how they're grouped into two categories.  One is the `break;'
behavior, which is commented accordingly and is where the
temptation to use `default:' in place of all these can cause
subtle code-generation bugs.  (Namely, forget to put explicit
code to code-gen an intrinsic in here, and the code-gen that
happens makes assumptions that might be subtly wrong.)  So these
cases all are known (or strongly believed ;-) to be ones where
the fall-through behavior is wanted.

The other is the `die horrible death at run time' (aka `abort()')
behavior, another place tempting to use `default:', where doing
so causes less problematic results.  (Namely, forget to put
explicit code here, and the code-gen crashes the compiler.)  These
cases are known (believed) to be ones that simply shouldn't happen
in this section of code.

By avoiding `default:' entirely, the risk of subtle code-generation
bugs is still present (when compiling without asking for warnings),
but the typical case (compiling via recent gcc's with -W -Wall)
is that you get warned at *build* time, not run time compiling code
that happens to use the intrinsic in question, that the intrinsic
does not have its own code-gen code.

I believe that's a good example of when to avoid `default:',
though a better one is where the default `break;' behavior is
valid for forgotten cases, such as when finding optimal cases
to call out, for which the default behavior should still work.

Whenever the whole `switch' statement can be eliminated, e.g.
it's searching for cases to optimize, `default: break;' should
be fine.  Otherwise, some careful thought needs to go into
how to balance things like compile-time optimizations, possibility
of forgetting cases, results of forgotten cases (low or high
risk of doing bad things), and so on.

Being a bit of a language pedant, I'd love a way to specify that
the default behavior for a particular `switch' is to abort
rather than break *without* disabling compiler warnings about
unhandled cases (e.g. maybe a keyword like `missing:' in place
of `default:').  That'd be what I'd want to use in place of
what ffecom_expr_intrinsic_ currently does.  But C is not a
language for pedants.  ;-)

Actually, it occurs to me that the code in question could be
made *slightly* safer by inserting a `default:' in with the
aborting `case' statements, but conditionalizing it on being
compiled by a non-gcc compiler, so gcc continues to warn
about it, while other compilers (which might not) know to
make the program (g77, or f771 really) crash and burn rather
than fall through for missing cases.

        tq vm, (burley)

P.S. A note about Jeff Law's response: yes, it's true that
trying to be too pedantic about this stuff can be problematic,
especially when not all cases can be known to the author
of the code (e.g. where a language front-end adds more cases
via dynamic build-time configuration, true for C++ for example).

In other situations (mostly), it's important to remember that
the "cost of maintenance" of adding a new case globally (i.e.
adding a new tree node) isn't just limited to whether one
must hand-edit `switch' statements to avoid new warnings.

If those `switch' statements are properly modified *now*, then
the cost of maintenance might *appear* to be higher when new
nodes are added.

But the cost of maintenance could be *much* higher than if
we don't modify the statements correctly now, if the impact is
that we can't easily find where new `case' statements *should*
be added for, say, a new tree node, resulting in incorrect
code being generated.

In summary, among the *lowest* factors of "cost of maintenance"
is anything that requires a knowledgable programmer to analyze
and hand-edit code based on automatically produced reports
highlighting possible bugs.  Make that kind of work "go away"
by eliminating the reports, and you increase the risk of
incurring the *highest* factors of "cost of maintenance",
for example, less-knowledgable programmers trying to debug,
perhaps months later, the output of the application in question.

Maybe the most important point we're all making is this:

  Nobody should try to eliminate compiler warnings without
  knowing a *lot* about what they're doing.  If their work
  could truly be replaced via automation, then don't let them
  do it.