bugGNU Octave - Bugs: bug #41032, Command function parsing: does...

 
 

bug #41032: Command function parsing: does handle certain patterns involving commas as parentheses

Submitter:  None
Submitted:  Mon 30 Dec 2013 07:16:54 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Michael C. Grant Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.8.0
Operating System:  * Mac OS Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 07 Jan 2014 07:01:53 PM UTC, comment #18: 

Great! The additional tests make a lot of sense. Thanks for checking this in.

Michael C. Grant <mcgrant>
Tue 07 Jan 2014 06:59:06 PM UTC, comment #17: 

I added a few more tests and checked in this change:

  http://hg.savannah.gnu.org/hgweb/octave/rev/af8a70d6885c

Thanks for working on this and for making your second set of changes relative to the version I checked in.

John W. Eaton <jwe>
Group administrator
Tue 07 Jan 2014 12:29:03 AM UTC, comment #16: 

I'm attaching a new version of libinterp/parse-tree/lex.ll that fixes a couple of problems with whitespace, comment, and continuation parsing. Please replace the existing one...

If you want me to create a change set I can, but I'm hoping it's a bit easier for you---I'm having to test in a Homebrew interactive session which is different than the development branch.

I'm also attaching a test file, that I think is in the right format. I have verified that this new lex.ll file passes these tests. I hope tests are allowed to use 'assignin' because that's how I'm getting the internal arguments seen by the command.

(file #30204, file #30205)

Michael C. Grant <mcgrant>
Mon 06 Jan 2014 08:29:32 PM UTC, comment #15: 

Yeah, quoting both args should still be OK.  But the previous test had


close "all" hid"


John W. Eaton <jwe>
Group administrator
Mon 06 Jan 2014 08:20:15 PM UTC, comment #14: 

I'm glad you're having me do the tests, I've already discovered an issue with my handling of periods... you need to change

<COMMAND_START>([\.]|[^#% \t\r\n\,\;\"\'\(\[\{\}\]\)]+)

to

<COMMAND_START>([\.]|[^#% \t\r\n\.\,\;\"\'\(\[\{\}\]\)]+)

I'm constructing the tests now, and that's how I uncovered it :-)

Michael C. Grant <mcgrant>
Mon 06 Jan 2014 07:58:57 PM UTC, comment #13: 

Actually, the close.m script should see exactly the same results with


close "all" "hid"


and


close all hid


So that change probably wasn't necessary, strictly speaking. But at the same time, nobody would use the quotes in that context anyway, so your change makes the test more realistic/natural.

Michael C. Grant <mcgrant>
Mon 06 Jan 2014 07:50:47 PM UTC, comment #12: 

I changed the line


%!error <expecting argument to be "all hidden"> close "all" hid"


in the close.m tests to


%!error <expecting argument to be "all hidden"> close all hid


That quoting in the original version of the file looks strange to me.  I don't think it should work.  Or why it did previously.

John W. Eaton <jwe>
Group administrator
Mon 06 Jan 2014 07:46:20 PM UTC, comment #11: 

I'll be happy to put together some tests. I don't have anything formal for you---and it seems like using CVX for tests would be overkill. Nevertheless I will construct a series of testing scenarios: some artificial meant to test each parse action, and then some common choices from CVX itself.

Which test did you have to modify?

Michael C. Grant <mcgrant>
Mon 06 Jan 2014 07:41:22 PM UTC, comment #10: 

Michael,

Thanks for working on this problem and sending me modified lex.h and lex.ll files.  I checked in the changes here:

  http://hg.savannah.gnu.org/hgweb/octave/rev/615fdd2238c1

It seems to work for me, though I did have to modify one test but I don't think that's a problems since it seemed like it was using the wrong syntax to me anyway.

Do you have a set of tests or test script that you can share?  I can take care of converting them to the format required for Octave's internal tests.


John W. Eaton <jwe>
Group administrator
Mon 30 Dec 2013 04:15:23 PM UTC, comment #9: 

The command "syms.m" function in the Symbolic Math Toolbox (http://www.mathworks.com/help/symbolic/syms.html) relies on the ability to accept arguments in a function declaration form; e.g.


f(x1,x2,x3)


and it parses its arguments with standard MATLAB code (find,regexp,etc.) My expressions can potentially be more complex than this: I allow expressions in those argument positions, which are evaluated in the parent workspace. But I don't try to do anything too intelligent: I just break apart the argument list by commas and evaluate each resulting token.

Michael C. Grant <mcgrant>
Mon 30 Dec 2013 03:59:04 PM UTC, comment #8: 

Actually, correction: I had CVX working fully with MATLAB 6.1. Someone tried to get it working with 6.0, but had a problem unrelated to MATLAB's command parsing (a lack of 'strfind', to be specific).

Michael C. Grant <mcgrant>
Mon 30 Dec 2013 03:53:57 PM UTC, comment #7: 

You are right---this is undocumented behavior. It is also very stable undocumented behavior. I've been relying upon it since Matlab 6.5, and I'm pretty sure it was there for some time before that. (I had CVX partially working on older versions of MATLAB, but ultimately had to ditch the effort because of other limitations).

I'm going to make a guess that MATLAB depends upon this behavior itself, perhaps in the Symbolic Math Toolbox or another. I will try and find out.

Michael C. Grant <mcgrant>
Mon 30 Dec 2013 03:43:40 PM UTC, comment #6: 

FWIW, there doesn't seem to be any mention of the strange rules about brackets here:

http://www.mathworks.com/help/matlab/matlab_prog/command-vs-function-syntax.html

It seems to me that you have been relying on undocumented Matlab features.  My experience says that crazy freaking rabbit holes are the norm when dealing with undocumented Matlab, possibly because the behavior is not intentional but just an accident of the implementation.  Or it's possible that the behavior is intentional, but the person who documented it didn't understand it any better than we do.  Who knows?

Can you find documentation that explains the behavior?  Can you get TMW to explain and/or document it more fully?  Without that, who's to say that it won't just change arbitrarily in some future Matlab release?

John W. Eaton <jwe>
Group administrator
Mon 30 Dec 2013 08:33:21 AM UTC, comment #5: 

OK, this is a crazy freaking rabbit hole. Here are some observations. When I say "bracket" below, I mean a parenthesis, curly brace, OR square bracket. Furthermore, in the examples, I'm using the following modified version of the command_test1 function:


function command_test1(varargin)
disp(sprintf('|%s|',varargin{:}));


  • MATLAB sometimes allows the first token to be an operator, sometimes not. I believe you are aware of those rules because I saw a reference to them in another bug report/patch. This is irrelevant to me personally.


  • The first token of a command cannot begin with a parenthesis (open or closed). However, the first argument can begin with any other bracket, and subsequent tokens can indeed begin with a parenthesis.



>> command_test1 (text]
 command_test1 (text]
                    |
Error: Unbalanced or unexpected parenthesis or bracket.

>> command_test1 [text]
|[text]|
>> command_test1 {text}
|{text}|
>> command_test1 {text1} (text2)
|{text1}||(text2)|


  • Except for the above distinction brackets are treated identically and interchangeably. In other words, an open bracket is matched by any close bracket. (It gets weirder; stay tuned.)

 

>> command_test1 {text]
|{text]|


  • Whenever the total number of open brackets equals the number of close brackets, we have what might be considered reasonable behavior: 1) single quotes are treated as string delimiters, and must be matched; and 2) whitespace separates tokens. Note that the quotes surrounding a quote-delimited string are removed, even if the string forms just a part of a command token.



>> command_test1 {'('[')')')'test'n'{}
 command_test1 {'('[')')')'test'n'{}
               |
Error: A MATLAB string constant is not terminated properly.
 >> command_test1 {([)))'test''n'{}
|{([)))test'n{}|


  • Whenever the total number of open brackets is less OR greater (yes, either way) than the number of close brackets, the text is passed unprocessed, including single quotes and whitespace.



>> command_test1 {'('[')')')'test''n'{'}
|{'('[')')')test'n{'}|


  • Commands can be split across lines with '...'. No matter the state of the brackets, the token ends. The ellipsis may be followed with arbitrary text, which is dropped.



>> command_test1 {test1}...test2
{test3}
|{test1}||{test3}|


Michael C. Grant <mcgrant>
Mon 30 Dec 2013 07:44:15 AM UTC, comment #4: 

Commands can indeed be continued across lines. I do not have a precise rule set for you, but I will look. It does appear that parentheses behave as a sort of relaxed quotation operator. When single quotes are used, they must be matched, or an error will occur; but if parentheses are not matched, the token ends without error. On the other hand, quotes within parentheses need not be matched.

Here are some strange corner cases.

>> command_test1 this'is
 command_test1 this'is
               |
Error: A MATLAB string constant is not terminated properly.

>> command_test1 this'is'
{thisis}
>> command_test1 this(is
{this(is}
>> command_test1 this(is)
{this(is)}
>> command_test1 this(is ....
a test
{this(is }{a}{test}
>> command_test1 this(')
{this(')}
>> command_test1 this('''')
{this('''')}
>> command_test1 this('
{this('}



Michael C. Grant <mcgrant>
Mon 30 Dec 2013 07:33:04 AM UTC, comment #3: 

Octave splits commands on whitespace.  I wasn't aware of the (strange, IMO) rules involving parens.  Can you precisely state what the rules are?

If you want to experiment with different rules for splitting tokens in commands, the place to look is libinterp/parse-tree/lex.ll.  Looking now, I noticed that COMMAND_START is not an exclusive start state, though perhaps it should be.

Can commands be continued across lines?  If so, in what contexts?

John W. Eaton <jwe>
Group administrator
Mon 30 Dec 2013 07:27:55 AM UTC, comment #2: 

I apologize, but it would seem I misspelled a verbatim tag. I'm including a reformatted version of the text here. If someone wishes to edit this post to consolidate, I would appreciate it.

I'm trying to port my optimization modeling software CVX (http://cvxr.com/cvx)
to Octave. 3.8.0 is a huge leap forward and I believe I'm very close to being
able to do this. I can't tell you how often I am asked about this, and I'm
pleased that it seems that soon it will be possible to do so.

I use MATLAB's command syntax extensively to emulate an embedded modeling
language. Unfortunately, Octave 3.8 is unable to handle some of the
constructions constructions. For instance, to declare a 2-D matrix variable of
size M times N, I allow the user to enter the following command:

    variable A(M,N)

MATLAB slurps in 'A(M,N)' as a single token, which I can then parse to extract
the name and dimension information. I can include whitespace within the
parentheses, and MATLAB will still preserve the result as a single token.
Unfortunately, Octave's behavior is different: not only does it ignore
parentheses grouping, but it cannot handle commas at all. Actually, this
second issue is understandable given the first, since the comma has an
important syntactic purpose in MATLAB.

To demonstrate, consider the following code:

    function command_test1(varargin)
    disp(sprintf('{%s}',varargin{:}))

Here is some sample output:

    octave:63> command_test1 this is a test
    {this}{is}{a}{test}
    octave:64> command_test1 this(nx) is a test
    {this(nx)}{is}{a}{test}

So far, so good; this matches MATLAB precisely. However, this differs between
Octave and MATLAB:

    octave:65> command_test1 this( nx ) is a test
    {this(}{nx}{)}{is}{a}{test}

MATLAB:

    >> command_test1 this( nx ) is a test
    {this( nx )}{is}{a}{test}

Furthermore, if I try this, I get a parse error:

    octave:64> command_test1 this(nx,ny) is a test
    parse error:
     syntax error
    command_test1 this(nx,ny) is a test
                               ^

MATLAB:

    command_test1 this(nx,ny) is a test
    {this(nx,ny)}{is}{a}{test}

I have found that employing quotes is a workaround, but this behavior differs
between MATLAB and Octave as well. And I'm afraid it would defeat a
fundamental purpose of my code to require it:
Octave:

    octave:67> command_test1 "this(,)" is a test
    {this(,)}{is}{a}{test}
    octave:68> command_test1 'this(,)' is a test
    {this(,)}{is}{a}{test}

MATLAB:

    command_test1 "this( nx )" is a test
    {"this( nx )"}{is}{a}{test}
    command_test1 'this( nx )' is a test
    {this( nx )}{is}{a}{test}

The quoting behavior causes no conflicts with my code (and I understand the
motivation for supporting double quotes for strings). But the lack of
parenthesis support does. I would much appreciate it if you would consider
adopting a modification that brings Octave closer to parity here. I'll see if
I can't dig into the parsing and do a patch.

Thanks
Michael

Michael C. Grant <mcgrant>
Mon 30 Dec 2013 07:26:45 AM UTC, comment #1: 

It looks like the bug tracker is not displaying a large portion of your report because of a botched verbatim tag.

Here it is, as best as I can tell from the email sent by the tracker:

Details:

I'm trying to port my optimization modeling software CVX (http://cvxr.com/cvx)
to Octave. 3.8.0 is a huge leap forward and I believe I'm very close to being
able to do this. I can't tell you how often I am asked about this, and I'm
pleased that it seems that soon it will be possible to do so.

I use MATLAB's command syntax extensively to emulate an embedded modeling
language. Unfortunately, Octave 3.8 is unable to handle some of the
constructions constructions. For instance, to declare a 2-D matrix variable of
size M times N, I allow the user to enter the following command:


variable A(M,N)


MATLAB slurps in 'A(M,N)' as a single token, which I can then parse to extract
the name and dimension information. I can include whitespace within the
parentheses, and MATLAB will still preserve the result as a single token.
Unfortunately, Octave's behavior is different: not only does it ignore
parentheses grouping, but it cannot handle commas at all. Actually, this
second issue is understandable given the first, since the comma has an
important syntactic purpose in MATLAB.

To demonstrate, consider the following code:


function command_test1(varargin)
disp(sprintf('{%s}',varargin{:}))


Here is some sample output:


octave:63> command_test1 this is a test
{this}{is}{a}{test}
octave:64> command_test1 this(nx) is a test
{this(nx)}{is}{a}{test}


So far, so good; this matches MATLAB precisely. However, this differs between
Octave and MATLAB:

Octave:


octave:65> command_test1 this( nx ) is a test
{this(}{nx}{)}{is}{a}{test}


MATLAB:


>> command_test1 this( nx ) is a test
{this( nx )}{is}{a}{test}


Furthermore, if I try this, I get a parse error:

Octave:


octave:64> command_test1 this(nx,ny) is a test
parse error:

  syntax error

>>> command_test1 this(nx,ny) is a test
                            ^


MATLAB:


>> command_test1 this(nx,ny) is a test
{this(nx,ny)}{is}{a}{test}


I have found that employing quotes is a workaround, but this behavior differs
between MATLAB and Octave as well. And I'm afraid it would defeat a
fundamental purpose of my code to require it:

Octave:


octave:67> command_test1 "this(,)" is a test
{this(,)}{is}{a}{test}
octave:68> command_test1 'this(,)' is a test
{this(,)}{is}{a}{test}


MATLAB:


>> command_test1 "this( nx )" is a test
{"this( nx )"}{is}{a}{test}
>> command_test1 'this( nx )' is a test
{this( nx )}{is}{a}{test}


The quoting behavior causes no conflicts with my code (and I understand the
motivation for supporting double quotes for strings). But the lack of
parenthesis support does. I would much appreciate it if you would consider
adopting a modification that brings Octave closer to parity here. I'll see if
I can't dig into the parsing and do a patch.

Thanks
Michael

John W. Eaton <jwe>
Group administrator
Mon 30 Dec 2013 07:16:54 AM UTC, original submission:  

I'm trying to port my optimization modeling software CVX (http://cvxr.com/cvx) to Octave. 3.8.0 is a huge leap forward and I believe I'm very close to being able to do this. I can't tell you how often I am asked about this, and I'm pleased that it seems that soon it will be possible to do so.

I use MATLAB's command syntax extensively to emulate an embedded modeling language. Unfortunately, Octave 3.8 is unable to handle some of the constructions constructions. For instance, to declare a 2-D matrix variable of size M times N, I allow the user to enter the following command:


variable A(M,N)
-verbatim

MATLAB slurps in 'A(M,N)' as a single token, which I can then parse to extract the name and dimension information. I can include whitespace within the parentheses, and MATLAB will still preserve the result as a single token. Unfortunately, Octave's behavior is different: not only does it ignore parentheses grouping, but it cannot handle commas at all. Actually, this second issue is understandable given the first, since the comma has an important syntactic purpose in MATLAB.

To demonstrate, consider the following code:

+verbatim+
function command_test1(varargin)
disp(sprintf('{%s}',varargin{:}))


Here is some sample output:

octave:63> command_test1 this is a test
{this}{is}{a}{test}
octave:64> command_test1 this(nx) is a test
{this(nx)}{is}{a}{test}


So far, so good; this matches MATLAB precisely. However, this differs between Octave and MATLAB:

Octave:

octave:65> command_test1 this( nx ) is a test
{this(}{nx}{)}{is}{a}{test}


MATLAB:

>> command_test1 this( nx ) is a test
{this( nx )}{is}{a}{test}


Furthermore, if I try this, I get a parse error:

Octave:

octave:64> command_test1 this(nx,ny) is a test
parse error:

  syntax error

>>> command_test1 this(nx,ny) is a test
                            ^


MATLAB:

>> command_test1 this(nx,ny) is a test
{this(nx,ny)}{is}{a}{test}


I have found that employing quotes is a workaround, but this behavior differs between MATLAB and Octave as well. And I'm afraid it would defeat a fundamental purpose of my code to require it:

Octave:

octave:67> command_test1 "this(,)" is a test
{this(,)}{is}{a}{test}
octave:68> command_test1 'this(,)' is a test
{this(,)}{is}{a}{test}


MATLAB:

>> command_test1 "this( nx )" is a test
{"this( nx )"}{is}{a}{test}
>> command_test1 'this( nx )' is a test
{this( nx )}{is}{a}{test}


The quoting behavior causes no conflicts with my code (and I understand the motivation for supporting double quotes for strings). But the lack of parenthesis support does. I would much appreciate it if you would consider adopting a modification that brings Octave closer to parity here. I'll see if I can't dig into the parsing and do a patch.

Thanks
Michael



Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #30204:  command.tst added by mcgrant (2KiB - application/octet-stream)
file #30205:  lex.ll added by mcgrant (83KiB - application/octet-stream)
file #30197:  command.diff added by None (10KiB - application/octet-stream - Proposed changeset.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mcgrant (Posted a comment)
  • -email is unavailable- added by jwe (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-01-07 mcgrant Attached File- Added command.tst, #30204
        Attached File- Added lex.ll, #30205
    2014-01-06 jwe StatusNone Fixed
        Open/ClosedOpen Closed
    2014-01-05 None Attached File- Added command.diff, #30197

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code