[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

[Suharto Anggono Suharto Anggono]

In 'conformMethod', there is another instance of
omittedSig & <something about signature[omittedSig]> .
It just affects error message.

Original:
    if(any(is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
        bad <- omittedSig & is.na(match(signature[omittedSig], c("ANY", "missing")))

After r76756:
    if(any(iiN <- is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
        bad <- omittedSig & iiN


----------------------------------------------
>>>>> Martin Maechler 
>>>>>     on Sat, 29 Jun 2019 12:05:49 +0200 writes:

>>>>> Martin Maechler 
>>>>>     on Sat, 29 Jun 2019 10:33:10 +0200 writes:

>>>>> peter dalgaard 
>>>>>     on Fri, 28 Jun 2019 16:20:03 +0200 writes:

    >>> > On 28 Jun 2019, at 16:03 , Martin Maechler <maechler using stat.math.ethz.ch> wrote:
    >>> > 
    >>> >>>>>> Henrik Bengtsson 
    >>> >>>>>>    on Thu, 27 Jun 2019 16:00:39 -0700 writes:
    >>> > 
    >>> >> Using:
    >>> >> 
    >>> >> untrace(methods::conformMethod)
    >>> >> at <- c(12,4,3,2)
    >>> >> str(body(methods::conformMethod)[[at]])
    >>> >> ## language omittedSig <- omittedSig && (signature[omittedSig] != "missing")
    >>> >> cc <- 0L
    >>> >> trace(methods::conformMethod, tracer = quote({
    >>> >>  cc <<- cc + 1L
    >>> >>  print(cc)
    >>> >>  if (cc == 31) {  ## manually identified
    >>> >>    untrace(methods::conformMethod)
    >>> >>    trace(methods::conformMethod, at = list(at), tracer = quote({
    >>> >>      str(list(signature = signature, mnames = mnames, fnames = fnames))
    >>> >>      print(ls())
    >>> >>      try(str(list(omittedSig = omittedSig, signature = signature)))
    >>> >>    }))
    >>> >>  }
    >>> >> }))
    >>> >> loadNamespace("oligo")
    >>> >> 
    >>> >> gives:
    >>> >> 
    >>> >> Untracing function "conformMethod" in package "methods"
    >>> >> Tracing function "conformMethod" in package "methods"
    >>> >> Tracing conformMethod(signature, mnames, fnames, f, fdef, definition)
    >>> >> step 12,4,3,2
    >>> >> List of 3
    >>> >> $ signature: Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" "array"
    >>> >>  ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value"
    >>> >>  ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" "methods" "methods"
    >>> >> $ mnames   : chr [1:2] "object" "value"
    >>> >> $ fnames   : chr [1:4] "object" "subset" "target" "value"
    >>> >> [1] "f"          "fdef"       "fnames"     "fsig"       "imf"
    >>> >> [6] "method"     "mnames"     "omitted"    "omittedSig" "sig0"
    >>> >> [11] "sigNames"   "signature"
    >>> >> List of 2
    >>> >> $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE
    >>> >> $ signature : Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" "array"
    >>> >>  ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value"
    >>> >>  ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" "methods" "methods"
    >>> >> Error in omittedSig && (signature[omittedSig] != "missing") :
    >>> >>  'length(x) = 4 > 1' in coercion to 'logical(1)'
    >>> >> Error: unable to load R code in package 'oligo'
    >>> >> 
    >>> > 
    >>> > Thank you, Henrik, nice piece of using trace() .. and the above
    >>> > is useful for solving the issue --  I can work with that.
    >>> > 
    >>> > I'm  already pretty sure the wrong code starts with
    >>> > 
    >>> >    omittedSig <- sigNames %in% fnames[omitted] # ....

    >> my  "pretty sure"  statement above has proven to be wrong ..

    >>> > -------------
    >>> > 
    >>> 
    >>> I think the intention must have been that the two "ANY" signatures should change to "missing". 

    >> Definitely.

    >>> However, with the current logic that will not happen, because
    >>> 
    >>> > c(F,T,T,F) &&  c(T,T)
    >>> [1] FALSE
    >>> 
    >>> Henrik's non-fix would have resulted in
    >>> 
    >>> > c(F,T,T,F) &  c(T,T)
    >>> [1] FALSE  TRUE  TRUE FALSE
    >>> 
    >>> which is actually right, but only coincidentally due to recycling of c(T,T). Had it been c(F,T) then it would have been expanded to c(F,T,F,T) which would be the opposite of what was wanted.
    >>> 
    >>> Barring NA issues, I still think 
    >>> 
    >>> omittedSig[omittedSig] <- (signature[omittedSig] != "missing")
    >>> 
    >>> should do the trick.

    >> yes, (most probably).  I've found a version of that which should
    >> be even easier to "read and understand", in  svn commit 76753 :

    >> svn diff -c 76753 src/library/methods/R/RMethodUtils.R

    >> --- src/library/methods/R/RMethodUtils.R    (Revision 76752)
    >> +++ src/library/methods/R/RMethodUtils.R    (Revision 76753)
    >> @@ -342,8 +342,7 @@
    >> gettextf("formal arguments (%s) omitted in the method definition cannot be in the signature", bad2),
    >> call. = TRUE, domain = NA)
    >> }
    >> -    else if(!all(signature[omittedSig] == "missing")) {
    >> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
    >> +    else if(any(omittedSig <- omittedSig & signature != "missing")) {


    >> BTW:  I've marked this --- and the  runmed() seg.fault + na.action
    >> change ---  as something to be added to R 3.6.1 patched,  as I
    >> deemed I should obey the "code freeze" rule in both cases.

    >> Martin

    > Hmm... I think we got a 'Neverending Story' here -- because it
    > seems, both Peter and I were wrong in thinking that it's a good
    > idea to change "missing" to "ANY" here ...
    > ((or if that *was* correct, that needs to entail more changes
    > happening during setMethod(.) {conformMethod() is only called in
    > one place in our code base, namely from setMethod()} :

as a matter of fact, I've been brave for now, left the change to
conformMethod()  and started to fix the constructed .local()
calls which are created in  conformMethod()'s sibling,
which is rematchDefinition().

It seems that this builds e.g. Matrix {with its gazillion
setMethod()s} and that continues to run its own tests.  OTOH,
Matrix may not trigger the situations that are dealt with here
at all, as the signature() are rarely longer than three, and at
some point in time I had made long passes through the package in
order to "minimize" the .local() calls.

--> svn commit 76756  (in addition to 76753, mentioned earlier)
    now has rematchDefinition() changes

I would be positively surprised if (but can imagine that) this
had no affect on CRAN / Bioconductor packages.

Still, these two changes seem to achieve what both the comments
and the documentation of  conformMethod() and rematchDefinition()
suggest should happen.

Of course, I'd really be happy if people with S4 packages would
check them with an R-devel version with svn rev >=  76756
and report problems they see.
I do imagine effects, and would expect that bugs in current code
become visible where they had not done so previously.

Martin