bugGNU Octave - Bugs: bug #46459, Interpreter allows number...

 
 

bug #46459: Interpreter allows number assignment to "end" key word within array brackets

Submitter:  None
Submitted:  Mon 16 Nov 2015 08:52:26 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  jwe
Originator Name:  Wolfgang F. Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.0.0
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 28 Feb 2016 03:50:05 PM UTC, comment #13: 

Everything works for me.  Closing report.

Rik <rik5>
Group administrator
Sat 27 Feb 2016 05:35:03 PM UTC, comment #12: 

I checked in this additional change.  It also reworks the original patch.

http://hg.savannah.gnu.org/hgweb/octave/rev/105224df2330

John W. Eaton <jwe>
Group administrator
Fri 26 Feb 2016 08:08:58 PM UTC, comment #11: 

@jwe: The patch works for the original reporter's case.

Do you want to close this report, or also deal with the assignin bug that Mike found?  I'm not that bothered by it (see comment #3) and would let it pass.

Sample code


x = 5:-1:1;
assignin ("base", "end", 2);
x(end)
4



Rik <rik5>
Group administrator
Fri 26 Feb 2016 06:33:56 PM UTC, comment #10: 

I checked in the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/9ca194f7a858

John W. Eaton <jwe>
Group administrator
Fri 26 Feb 2016 04:10:46 PM UTC, comment #9: 

Ah, there is a bit of confusion there.  Using "end" as an object index is not the same as "end" being a keyword to mark the end of a block of code (like a for loop) because in that context it is converted into a function call.  We still need to prevent assignment to "end" in that context, but it can't be done at that point in the lexer.  It needs to be done when the assignment statement is constructed.  I'll see if I can fix this one.

John W. Eaton <jwe>
Group administrator
Thu 18 Feb 2016 10:09:30 PM UTC, comment #8: 

Thanks for taking charge of this, jwe.

I think the code already checks that the LHS of an assignment is not a keyword.  The hitch is that the code to check if something is a keyword has a special case to ignore "end" in this context.


octave_base_lexer::is_keyword_token (const std::string& s)
...
case end_kw:
  if (inside_any_object_index ()
      || (defining_func
          && ! (looking_at_return_list
                || parsed_function_name.top ())))
    {
      at_beginning_of_statement = previous_at_bos;
      return 0;
    }


(When I made the comment about performance, I hadn't realised that the parser replaces all variable names with references to the particular symbol object; I assumed the identifier strings were processed at runtime.)

Lachlan Andrew <lachlan>
Thu 18 Feb 2016 05:11:00 AM UTC, comment #7: 

I could be wrong, but I don't think it should be too hard to check that the LHS of an assignment is not a keyword.  It should be possible to do this when the assignment expression is constructed, so there should be no performance penalty for evaluation.

John W. Eaton <jwe>
Group administrator
Wed 17 Feb 2016 11:14:54 PM UTC, comment #6: 

Anyone working on this may want to consider bug #33637 at the same time.

Lachlan Andrew <lachlan>
Sat 21 Nov 2015 03:46:03 AM UTC, comment #5: 

Doing a complete check against all keywords in the interpreter would not only complicate the code but would also degrade performance.

In some ways, allowing assignment via assignin could be considered a useful feature.  The assignment within the index brackets could/should be checked at parse time, IMHO.

I agree with Wolfgang that the end+1/end=1 typo bug is plausible enough to justify a fix, even if just a hack.

$0.02

Lachlan Andrew <lachlan>
Tue 17 Nov 2015 08:07:07 PM UTC, comment #4: 

It could be source of octave code being difficult to debug when you don't expect such behavior. Simple typo will lead to e.g. a(end=1)=x instead of a(end+1)=x and no error is reported.

WF

Anonymous
Tue 17 Nov 2015 07:19:40 PM UTC, comment #3: 

@Mike: The assignin() test seems to suggest that the check for keywords is being done at the parser level rather than at the level of the symbol table and interpreter.  Maybe we need checking in both places, or maybe we should move all checks to a single location in order to avoid code redundancy.  That location would have to be on the interpreter side as it walks through the tree of commands.

Or, we acknowledge that this is an oddity, because it seems like a lot of work to fix when it is a simple enough code construct to avoid.

Rik <rik5>
Group administrator
Tue 17 Nov 2015 06:01:36 PM UTC, comment #2: 

It appears that "end" and "for" can also be assigned via assignin:


assignin ("base", "end", 2);
assignin ("base", "for", 1);


although "for" still cannot be used as an index.

Mike Miller <mtmiller>
Group Member
Tue 17 Nov 2015 05:24:27 PM UTC, comment #1: 

Confirmed.

'end' is a reserved keyword and it shouldn't be possible to assign to it.  This is the case at the command line as shown below.  I also used another keyword, 'for' to show that in this case the interpreter is consistent and does not allow the assignment.


octave:3> x = [1 2 3];
octave:4> end = 3
parse error:

  syntax error

>>> end = 3
      ^

octave:4> for = 5
parse error:

  syntax error

>>> for = 5
        ^

octave:4> x(end=2)
ans =  2
octave:5> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        ans         1x1                          8  double
        end         1x1                          8  double
        x           1x3                         24  double

Total is 5 elements using 40 bytes

octave:6> x(for=2)
parse error:

  syntax error

>>> x(for=2)



Rik <rik5>
Group administrator
Mon 16 Nov 2015 08:52:26 PM UTC, original submission:  

Whithin the bracketing of arrays the interpreter allows to assign a value to the "end" key word. Matlab does not allow this behavior (was only able to test with 2010a).


a=[1 1 1]
a =

   1   1   1

>> a(end)
ans =  1
>> a(end)=3
a =

   1   1   3

>> a(end=1)=0
a =

   0   1   3

>> a(end)
ans = 0



I was searching if this bug already has been reported but could not find a matchinng submit.

WF

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-28 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2016-02-26 jwe StatusConfirmed Ready For Test
    2016-02-18 jwe Assigned toNone jwe
    2015-11-17 rik5 StatusNone Confirmed
        Operating SystemGNU/Linux Any

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code