From maechler at stat.math.ethz.ch  Sat Jul 22 18:20:11 2017
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Sat, 22 Jul 2017 18:20:11 +0200
Subject: [Rd] [PATCH] Fix missing break
In-Reply-To: <2491471.UUKbAB2nFm@x2>
References: <60035856.27PnXlZqtm@x2> <4241711.WyxJnDo7EK@x2>
 <22897.47489.517803.232885@stat.math.ethz.ch>
 <2491471.UUKbAB2nFm@x2>
Message-ID: <22899.31547.640743.278243@stat.math.ethz.ch>

>>>>> Steve Grubb <sgrubb at redhat.com>
>>>>>     on Fri, 21 Jul 2017 13:53:12 -0400 writes:

    > Hello Martin,
    > On Friday, July 21, 2017 4:21:21 AM EDT Martin Maechler wrote:
    >> I have now created an account for you.

    > Thanks. Is that the preferred method of transferring these patches?

in such a case, yes .. but don't ask for a full definition of "such
a case" ;-)
If the issue may be somewhat controversial and rather in the
spirit of "I don't like what R is doing here, and I think we
       	   should change ..."
we'd prefer it be posted here, first, in any case;  but you had
no such examples.	   

    >> >> In examples like the one below, if you have R code that shows symptoms,
    >> >> it would really help in the bug report.
    >> > 
    >> > I am hoping that we can look at the code as seasoned programmers and say
    >> > yeah, that is a bug.
    >> 
    >> I agree in this case.
    >> OTOH, it is exactly one of the case where the bug is not
    >> triggerable currently:
    >> 
    >> al <- formals(ls); length(al) <- 3
    >> 
    >> would trigger the bug... but you get an error message ".. vector .."
    >> and as I now found that is from a slightly misguided check:
    >> isVectorizable()  is not approriate here and should really be
    >> replaced by isList().
    >> 
    >> So .. indeed, your report will have triggered an improvement in
    >> the code, which I'm about to commit.

    > That's what it's all about.  :-)
 
    >> Thank you very much Steve!
    >> 
    >> > I run the code through Coverity and have quite a lot of
    >> > problems to tell you about.
    >> 
    >> I'm not the expert on static code analysis, but as a seasoned
    >> statistician (*and* from experience with other such analyses) I
    >> know that you always get false positives.

    > Absolutely. I weeded the report down to 15 issues to start with. There are 
    > also ways to annotate the code so that checkers dismiss something it would 
    > otherwise be inclined to report.

    >> >> Otherwise, someone else will have to analyze the code to decide whether
    >> >> it's a bug or missing comment.  That takes time, and if there are no
    >> >> known symptoms, it's likely to be assigned a low priority.  The sad
    >> >> truth is that very few members of R Core are currently actively fixing
    >> >> bugs.
    >> > 
    >> > That's a shame. I'd be happy to give the scan to people in core so they
    >> > can see what the lay of the land looks like.
    >> 
    >> (hmm... the above does look a teeny tiny bit arrogant in my
    >> eyes; but then I'm not a native English (nor "American" 
    >> speaker ...)

    > I apologize if that is the way it came across. "That's a shame" can also mean 
    > "That's unfortunate" because I was thinking that I spent some time fixing up 
    > patches that might not be wanted. However, I see that you have looked at the 
    > patches and I thank you for that.  :-)

    > The second sentence above is an honest offer. I'd be happy to send the output 
    > of the report off list (in case anything sensitive is listed). In this and the 
    > other patches I haven't sent, I'm just picking the low hanging fruit.

    > -Steve

Ok, thank you for the offer!   In general, we would prefer public
communication of such issues because it can help to spread the
volunteer work load a bit wider than only to R Core.   OTOH,
yes, there are important exceptions to this rule, as we know.

Martin



[Rd] [PATCH] Fix missing break
Martin Maechler