'unique' error message is printed despite silent=TRUE (PR#13547)

On 22/02/2009 4:38 PM, Wacek Kusnierczyk wrote:

Duncan Murdoch wrote:

hmm, why wouldn't you use something like

    DEBUG(x)

with DEBUG being a macro defined so that it's replacement is void
unless
a specific flag or environment variable is set specifically for the
purpose of debugging?  you would then avoid confusing users' code just
because one PrintValue has been inadvertently left in the sources.

But then we'd confuse developers, who would see a huge dump of messages
from every other debugging session, when they just wanted to see their
own, and who would be forced to wade through leftover never-used
DEBUG(x) calls in code when they were reading the source.

my point was not that such DEBUG statements should be left there in the
code for all eternity.  to the contrary, their role would be quite the
same as that of the PrintValue discussed here.  it would, however, be
easier to switch between printing and not printing such debugging
messages, and also easier to spot DEBUG statements inadvertently left in
the code. 

Sorry, I misunderstood.  Yes, that might be handy.

The main problem is agreeing on what macros to write, and what should
happen when the external flag is set. In my experience, people who are
good at debugging have long-established idiosyncratic habits, and are
just annoyed when things change.

For example, a number of people have suggested that compiles should
switch to optimization level 0 when compiling for debugging.  This
makes stepping through code easier, because (as far as I recall)
variables aren't optimized out, code isn't rearranged, etc.  But it
means some bugs change their behaviour:  and I really hate that.  So I
wouldn't mind if it were possible to request that, but I'd want to
make sure the default is to ask for debugging support without it:  I
don't want to waste my time looking at a different program when I'm
trying to track something down.

If we had DEBUG(x) which became PrintValue(x) when a certain flag was
set, I probably wouldn't use it, because it requires two things:  set
the flag as well as add the statement.  I'd find that just irritating.
(I rarely use PrintValue in any case:  most of the types of bugs I'm
looking for need Rprintf or REprintf instead.  So we'd need at least
three macros.)

This is not a common error:  as far as I know, there are no other
unintentional PrintValue calls anywhere in the source.  So I think the
current system (just take them out before committing) is working.

grep --include=*.c -R '\bPrintValue\b' src | wc -l

reports 21 occurrences of PrintValue, though of course i cannot say
anything about their being intentional or not unless i examine the
sources.  if they were DEBUGs, you'd know for sure they're not supposed
to stay there in a release version.

I did a quick examination of the source and I think the ones that
aren't commented out look intentional.  (I was following my rule 5 of
debugging:  look for similar errors elsewhere.)

it's just to wish that those who introduce debugging PrintValues
examined diffs carefully before their code is released.  given the size
of r sources (and their fairly ad hoc shape here and there), few others
than the author will know for sure whether the PrintValue is a debugger
or not?  apparently, no one has noticed in this case.  were it DEBUG
instead of PrintValue, it would suffice to run a grep to catch it.

People who commit any changes should examine them carefully, and in
general they do.  Sometimes things slip by.  In this case, the slip
was there for 5 years before anyone noticed it, and I don't think it
caused a lot of harm:  it was an error message that printed incorrectly.