From henr|k@bengt@@on @end|ng |rom gm@||@com  Tue Jun 25 07:16:53 2019
From: henr|k@bengt@@on @end|ng |rom gm@||@com (Henrik Bengtsson)
Date: Mon, 24 Jun 2019 22:16:53 -0700
Subject: [Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
In-Reply-To: <23824.41030.156424.792062@stat.math.ethz.ch>
References: <CAFDcVCQOWVr0p2e2HONOy9NvqPdpoAH=HPMOEP65K-N6UjhA2g@mail.gmail.com>
 <67D2E7FE-74E3-4379-B9FB-C4028AB808DF@gmail.com>
 <CAFDcVCTARMfrJRs-KiS5UiSo=SoU8+TgPh_KE+e-zFRR0eCSFQ@mail.gmail.com>
 <23824.41030.156424.792062@stat.math.ethz.ch>
Message-ID: <CAFDcVCTq-Zc49t9HrdVQB4VHhOtuaoOmpGnx9JsAy+_esene3w@mail.gmail.com>

**Maybe this bug needs to be understood further before applying the
patch because patch is most likely also wrong**

Because, from just looking at the expressions, I think neither the R
3.6.0 version:

omittedSig <- omittedSig && (signature[omittedSig] != "missing")

nor the patched version (I proposed):

omittedSig <- omittedSig & (signature[omittedSig] != "missing")

can be correct.  For a starter, 'omittedSig' is a logical vector.  We
see that 'omittedSig' is used to subset 'signature'.  In other words,
the length of 'signature[omittedSig]' and hence the length of
'(signature[omittedSig] != "missing")' will equal sum(omittedSig),
i.e. the length will be in {0,1,...,length(omittedSig)}.

The R 3.6.0 version with '&&' is not correct because '&&' requires
length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
to be the original intention.

The patched version with '&' is most likely not correct either because
'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
auto-expansion of either vector.  So, for the '&' version to be
correct, it basically requires that length(omittedSig) = length(LHS) =
length(RHS) = sum(omittedSig), which also sounds unlikely to be the
original intention.

Disclaimer: Please note that I have not at all studied the rest of the
function, so the above is just based on that single line plus
debugging that 'omittedSig' is a logical vector.

Martin, I don't have the time to dive into this further.  Though I did
try to see if it happened when one of oligo's dependencies were
loaded, but that was not the case. It kicks in when oligo is loaded.
FYI, I can also replicate your non-replicatation on another R 3.6.0
version. I'll see if I can narrow down what's different, e.g.
comparing sessionInfo():s, etc.  However, I want to reply with
findings above asap due to the R 3.6.1 release and you might roll back
the patch (since it might introduce other bugs as Peter mentioned).

/Henrik


On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>
> >>>>> Henrik Bengtsson via R-core
> >>>>>     on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>
>     > Thank you.
>     > To correct myself, I can indeed reproduce this with R --vanilla too.
>     > A reproducible example is:
>
>     > $ R --vanilla
>     > R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>     > ...
>     >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >> loadNamespace("oligo")
>     > Error in omittedSig && (signature[omittedSig] != "missing") :
>     > 'length(x) = 4 > 1' in coercion to 'logical(1)'
>     > Error: unable to load R code in package ?oligo?
>
>     > /Henrik
>
> Thank you Henrik, for the report, etc, but
> hmm... after loading the oligo package, almost 40 (non
> base+Recommended) packages have been loaded as well, which hence
> need to have been installed before, too ..
> which is not quite a "vanilla repr.ex." in my view
>
> Worse, I cannot reproduce :
>
>     > Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     > Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>     [1] "true"
>     > debugonce(conformMethod)
>     > loadNamespace("oligo")
>     <environment: namespace:oligo>
>     Warning messages:
>     1: multiple methods tables found for ?rowSums?
>     2: multiple methods tables found for ?colSums?
>     3: multiple methods tables found for ?rowMeans?
>     4: multiple methods tables found for ?colMeans?
>     > sessionInfo()
>     R Under development (unstable) (2019-06-20 r76729)
>
> (similarly with other versions of R >= 3.6.0).
>
> So, even though R core has fixed this now in the sources, it
> would be nice to have an "as simple as possible"  repr.ex. for this.
>
> Martin
>
>
>
>     > On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <pdalgd at gmail.com> wrote:
>     >>
>     >> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>     >>
>     >> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>     >>
>     >> -pd
>     >>
>     >> > On 22 Jun 2019, at 18:49 , Henrik Bengtsson <henrik.bengtsson at gmail.com> wrote:
>     >> >
>     >> > DISCLAIMER: I can not get this error with R --vanilla, so it only
>     >> > occurs when some other package is also loaded.  I don't have time to
>     >> > find to narrow that down for a reproducible example, but I believe the
>     >> > following error in R 3.6.0:
>     >> >
>     >> >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >> >> library(oligo)
>     >> > Error in omittedSig && (signature[omittedSig] != "missing") :
>     >> >  'length(x) = 4 > 1' in coercion to 'logical(1)'
>     >> > Error: unable to load R code in package 'oligo'
>     >> >
>     >> > is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>     >> > 'methods' package.  Here's the patch:
>     >> >
>     >> > $ svn diff src/library/methods/R/RMethodUtils.R &
>     >> > [1] 1062
>     >> > Index: src/library/methods/R/RMethodUtils.R
>     >> > ===================================================================
>     >> > --- src/library/methods/R/RMethodUtils.R (revision 76731)
>     >> > +++ src/library/methods/R/RMethodUtils.R (working copy)
>     >> > @@ -343,7 +343,7 @@
>     >> >              call. = TRUE, domain = NA)
>     >> >     }
>     >> >     else if(!all(signature[omittedSig] == "missing")) {
>     >> > -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>     >> > +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>     >> >         .message("Note: ", .renderSignature(f, sig0),
>     >> >                  gettextf("expanding the signature to include omitted
>     >> > arguments in definition: %s",
>     >> >                           paste(sigNames[omittedSig], "=
>     >> > \"missing\"",collapse = ", ")))
>     >> > [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>     >> >
>     >> > Maybe still in time for R 3.6.1?
>     >> >
>     >> > /Henrik
>     >> >
>     >> > ______________________________________________
>     >> > R-devel at r-project.org mailing list
>     >> > https://stat.ethz.ch/mailman/listinfo/r-devel
>     >>
>     >> --
>     >> Peter Dalgaard, Professor,
>     >> Center for Statistics, Copenhagen Business School
>     >> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>     >> Phone: (+45)38153501
>     >> Office: A 4.23
>     >> Email: pd.mes at cbs.dk  Priv: PDalgd at gmail.com
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
Henrik Bengtsson