Bug 17314 - CodeReview: Missing break handling SYMSXP
Summary: CodeReview: Missing break handling SYMSXP
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R 3.3.*
Hardware: Other Linux
: P5 enhancement
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2017-07-23 11:58 UTC by Steve Grubb
Modified: 2017-07-27 16:30 UTC (History)
1 user (show)

See Also:


Attachments
Patch to fi the issue (677 bytes, patch)
2017-07-23 11:58 UTC, Steve Grubb
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Grubb 2017-07-23 11:58:08 UTC
In src/main/subscript.c, there is a for loop where it loops
over names. If it matches it assigns the indx variable and breaks. This
break stops the for loop, but execution falls through to the default
handler. This leads to outputting an unwarranted error message.

A patch to fix this will be attached.
Comment 1 Steve Grubb 2017-07-23 11:58:59 UTC
Created attachment 2278 [details]
Patch to fi the issue
Comment 2 Martin Maechler 2017-07-24 09:39:05 UTC
Interesting.

After applying the "obvious" patch, we have

> x <- c(a=2, b=3); identical(2, x[[quote(a)]]); x[[quote(b)]] <- pi; x
[1] TRUE
       a        b 
2.000000 3.141593 

So the code in there originally was "thinking of" allowing to use symbols in place of strings.

It's  "kind of neat" but has been undocumented, and given the bug it has always(?) given the error message 

   Error in x[[quote(a)]] : invalid subscript type 'symbol' 

which most would think was appropriate.

From that point of a view, the best solution would be to just remove that whole case and directly emit the error message that it now does.

Opinions?
Comment 3 Martin Maechler 2017-07-24 09:46:56 UTC
(In reply to Martin Maechler from comment #2)

OTOH, this has worked for a long time (using another local function in subscript.c), possibly "forever" :

x <- c(a=2, b=3); x[[quote(b)]] <- pi; x
       a        b 
2.000000 3.141593 
> 

I don't think it is *documented* to work anywhere, so we could argue that is has never been part of R's API, and we are allowed to remove it, too.

In any case, either both `[[` and `[[<-`  should work with symbols, or both should fail.
Comment 4 Martin Maechler 2017-07-24 10:03:59 UTC
What also works (in current R and probably "forever"), from code reading
in src/main/subscript.c  is  `[` indexing with  "the missing object",
which is treated as equivalent to  "empty indexing" :


> M <- formals(I)$x # I think the shortest expression to get "the missing"
> missing(M)
[1] TRUE
> M
Error: argument "M" is missing, with no default
> ls.str()
M : <missing>
> x <- c(a=1, b=3)
> x[[M]]
Error in x[[M]] : invalid subscript type 'symbol'
> x[M]
a b 
1 3 
> x[]
a b 
1 3 
> x[M] <- 5; x
a b 
5 5 
>
Comment 5 Martin Maechler 2017-07-27 10:16:21 UTC
The bug is fixed, so I close now.
A new issue is the fact that the change introduced new semantics to R
which are not documented yet (`[[<-(x, <symsxp>, value)`)  and revealed pre-existing semantics also not documented  (`[[(x, <symsxp>)`).
Comment 6 Suharto Anggono 2017-07-27 16:30:26 UTC
(In reply to Martin Maechler from comment #4)
> What also works (in current R and probably "forever"), from code reading
> in src/main/subscript.c  is  `[` indexing with  "the missing object",
> which is treated as equivalent to  "empty indexing" :
> 
> 
> > M <- formals(I)$x # I think the shortest expression to get "the missing"
> > missing(M)
> [1] TRUE
> > M
> Error: argument "M" is missing, with no default
> > ls.str()
> M : <missing>
> > x <- c(a=1, b=3)
> > x[[M]]
> Error in x[[M]] : invalid subscript type 'symbol'
> > x[M]
> a b 
> 1 3 
> > x[]
> a b 
> 1 3 
> > x[M] <- 5; x
> a b 
> 5 5 
> >

str(as.list(quote(x[]))) gives this.
List of 3
 $ : symbol [
 $ : symbol x
 $ : symbol 

So, x[] is parsed as call to '[' with "the missing object" as the second argument.
That might explain the treatment of "the missing object" as index.