From ggrothendieck at gmail.com  Sun Nov  1 13:19:58 2009
From: ggrothendieck at gmail.com (Gabor Grothendieck)
Date: Sun, 1 Nov 2009 07:19:58 -0500
Subject: [Rd] Internal error in 'ls' for pathological environments
	(PR#14035)
In-Reply-To: <4AED6AC1.7000003@biostat.ku.dk>
References: <20091101040013.149F2282EF3D@mail.pubhealth.ku.dk> 
	<4AED6AC1.7000003@biostat.ku.dk>
Message-ID: <971536df0911010419g77bea545jcc30bd7165c82065@mail.gmail.com>

On Sun, Nov 1, 2009 at 6:02 AM, Peter Dalgaard <p.dalgaard at biostat.ku.dk> wrote:
> macrakis at alum.mit.edu wrote:
>>
>> nchar(with(list(2),ls())) gives an internal error. This is of course
>> a peculiar call (no names in the list), but the error is not caught
>> cleanly.
>>
>> It is not clear from the documentation whether with(list(2)...) is
>> allowable; if it is not, it should presumably give an error. If it is,
>> then
>> ls
>> shouldn't have problems with the resulting environment.
>>
>>> qq <- with(list(2),ls()) ? ? ? ? ? ? ? ? ? ? ? ? # An incorrect call (no
>>
>> names in list)
>>>
>>> nchar(qq)
>>
>> Error in nchar(qq) : 'getEncChar' must be called on a CHARSXP ?# ls
>> returned
>> a bad object
>>>
>>> qq
>>
>> [1]Error: 'getEncChar' must be called on a CHARSXP
>>>
>>> qq[1]
>>
>> [1]Error: 'getEncChar' must be called on a CHARSXP
>>>
>>> qq[2]
>>
>> [1] NA
>>
>> Apparently related:
>>
>>> with(list(a=1,2),ls())
>>
>> Error in ls() : 'getEncChar' must be called on a CHARSXP
>
> Thanks, yes, this looks wrong.
>
> Also, closer to the root cause:
>
>> eval(quote(ls()),list(a=1,2))
> Error in ls() : 'getEncChar' must be called on a CHARSXP
>
>
>> e <- evalq(environment(),list(2))
>> ls(e)
> [1]Error: 'getEncChar' must be called on a CHARSXP
>
> It is not quite clear that it should be allowed to have unnamed elements in
> lists used by eval (which is what with() uses internally). I suppose it
> should, since the intended semantics are unaffected in most cases. (I.e.,
> there is nothing really wrong with eval(quote(a+b), list(a=1,b=2,3,4)), and
> people may have been using such code unwittingly all over.)
>
> However, it IS a bug that we are creating ill-formed environments. The
> culprit seems to be that NewEnvironment (memory.c) is getting called in
> violation of its assumption that
>
> " This definition allows
> ?the namelist argument to be shorter than the valuelist; in this
> ?case the remaining values must be named already. ?(This is useful
> ?in cases where the entire valuelist is already named--namelist can
> ?then be R_NilValue.)
> "
>
> Removing the assumption from NewEnvironment looks like an efficiency sink,
> so I would suggest that we fix do_eval (eval.c) instead, effectively doing l
> <- l[names(l) != ""]. ?So in
>
> ? ?case LISTSXP:
> ? ? ? ?env = NewEnvironment(R_NilValue, duplicate(CADR(args)), encl);
> ? ? ? ?PROTECT(env);
> ? ? ? ?break;
>
> we need to replace the duplicate() call with something that skips the
> unnamed elements.
>
> The only side effect I can see from such a change is the resulting
> environments get a different length() than before. I'd say that if there are
> coder who actually rely on the length of an invalid environment, then they'd
> deserve what they'd get...
>

Why not go one further -- and have R crash That would really show them.



[Rd] Internal error in 'ls' for pathological environments (PR#14035)
Gabor Grothendieck