bugGNU Octave - Bugs: bug #47676, Cannot apply computed assignment...

 
 

bug #47676: Cannot apply computed assignment to a variable defined after the code was parsed (e.g., in a script)

Submitted by:  Carlo de Falco <cdf>
Submitted on:  Tue 12 Apr 2016 01:05:52 PM UTC  
 
Category: InterpreterSeverity: 4 - Important
Priority: 5 - NormalItem Group: Incorrect Result
Status: Patch SubmittedAssigned to: None
Originator Name: Open/Closed: Open
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 16 Jun 2016 06:23:34 AM UTC, comment #17:

In the future, it would also be good to have the syntax highlighting in the GUI editor show "word command" arguments in the "string" colour.

That will have to wait until the GUI has access to the parse tree (one of my medium term goals).

Lachlan Andrew <lachlan>
Project Member
Mon 13 Jun 2016 09:48:17 AM UTC, comment #16:

Oops. My previous patch had tabs in it. Try this one.

(file #37459)

Lachlan Andrew <lachlan>
Project Member
Mon 13 Jun 2016 04:40:04 AM UTC, comment #15:

Here is a patch that warns at parse time if A += B is parsed as A('+=', 'B'), or similar.

I'd be grateful for some feedback, especially on the trade-off between optimisation and readability.

This bug is more serious than I thought. I thought that it was only triggered if a script is called while another script or function is running.

However, it is also triggered if the variable is created from the command line before a script is run, if an old version of the script is cached. That means that the script could be run, throwing a warning but not failing due to some conditional code, and then be invoked again and fail without a warning.

(file #37458)

Lachlan Andrew <lachlan>
Project Member
Wed 08 Jun 2016 07:23:35 AM UTC, comment #14:

I just thought of another approach.

The parser known whether it is treating the string as a variable or a string, and what the first argument is. If the first argument starts with something that looks like an operator, it can give a warning that is clearer than "error: Csb(61,_): but Csb has size 1x1".

Some care would be needed with "/" so that "ls /home/joe" doesn't cause a warning. I'm not sure that much could be done about "ls /home".

This doesn't solve the problem, but it does avoid the worst part: silently giving the wrong result.

Lachlan Andrew <lachlan>
Project Member
Thu 05 May 2016 07:29:25 AM UTC, comment #13:

I wouldn't expect an to be thrown error in the case you describe. However, I would also expect the number to be correct, since JWE's comment #4 suggested that the '/' would be interpreted correctly and Csb would be "called" with no arguments and return the value of the variable.

Did you get an incorrect answer?

Lachlan Andrew <lachlan>
Project Member
Wed 04 May 2016 09:21:04 AM UTC, comment #12:

Lachlan,

I tried changing example_1.m to

I still don't see any error.
Did you expect this to behave differently?
What did you expect exactly?

Carlo de Falco <cdf>
Project Member
Wed 04 May 2016 02:14:46 AM UTC, comment #11:

Carlo, I agree that Matlab's response of throwing an error is better than Octave's of silently giving the wrong answer.

If Octave did "proceed with an unexpected value assigned to Csb", then I would consider it very much worse. However, this issue only affects the printing of the non-semicolon "Csb /2". If we actually do anything with the value (like using it in an assignment or function call), then I think Octave parses it correctly.

Lachlan Andrew <lachlan>
Project Member
Thu 14 Apr 2016 03:34:04 PM UTC, comment #10:

jwe,

Now that I think about it better, probably I can come up
with an example against the "#include" approach myself.

I tried the example lachlan suggested in comment #6 in both
Octave and Matlab and it fails in both cases,
but in different ways:

Matlab

Octave

While they both look bad, Matlab's result is slightly better
because at least the code does not proceed with an unexpected
value assigned to Csb.

Can we at least replicate this behaviour somehow?

Carlo de Falco <cdf>
Project Member
Thu 14 Apr 2016 01:31:21 PM UTC, comment #9:

Lachlan: I don't think reparsing scripts is an option either.

John W. Eaton <jwe>
Project Administrator
Thu 14 Apr 2016 01:30:13 PM UTC, comment #8:

Carlo: I don't think it would be a good idea to try to do something that works like an include statement without an actual #include (or similar) directive that will make the intent clear. I don't have time at the moment to provide an example, but if you want one I can probably come up with something.

Note that this problem exists with Matlab too. The list of operators is just different. For example, if you modify your script to have

then I think it will fail because Csb is not recognized as a variable when example_0 is parsed so the statement is converted to Csb ('+fact'). Then when it is evaluated, it can't find a Csb function to call. But if you write

it will do what you expect because with this spacing, the arguments look like an expression.

Yes, this is ridiculous.

Yes, we can fix it for Octave so that the OP= operators are handled the same as the OP operators. All I'm saying is that this will create an incompatibility. But perhaps it is more useful to recognize the OP= operators as operators (at least in some cases, depending on whitespace, same as other operators) than to ignore them for the sake of compatibility.

I always thought that parsing commands this way was stupid, but I didn't get to vote on what Matlab was going to do.

John W. Eaton <jwe>
Project Administrator
Thu 14 Apr 2016 11:31:54 AM UTC, comment #7:

jwe, lachlan,

I'm not really an expert about parsers
so forgive me if the following suggestion
does not make any sense.

IIUC the issue is that when a script is called
it is parsed completely before executing and
if it contains a call to another script, the
parser does not know what symbols are defined
in the second script when interpreting the first
one.

If this is the issue, would it make any sense to
consider the call to a script from within another
script as an "include" directive and passing to the
parser the full contents of the two combine scripts
to parse together?

BTW, is there any connection between this issue and
bug #38236 ?

Carlo de Falco <cdf>
Project Member
Thu 14 Apr 2016 01:55:50 AM UTC, comment #6:

Oops -- most of my comment was chopped off.

The gist was that if a script sets foo = zeros(100,100), then

would result in [1,1] with no warning that something was amiss.

As I've argued in the past, giving completely the wrong number with no warning makes Octave untrustworthy. IMHO, there should be a separate Item Group for this serious class of "non-errors", since "incorrect result" is a catch-all for any incorrect behaviour.

Perhaps the parse tree could flag that something was deemed to be a "function", and this could be checked by subsref of non-function types, either to give a more informative error or ideally to trigger a reparse of the code.

Another alternative would be to explicitly check after each script invocation whether new variables were created. If so, a reparse could again be triggered (if it is then possible to continue execution from that point).

Lachlan Andrew <lachlan>
Project Member
Thu 14 Apr 2016 01:19:41 AM UTC, comment #5:

JWE, your example

is really serious. If a script sets

Lachlan Andrew <lachlan>
Project Member
Wed 13 Apr 2016 12:08:36 PM UTC, comment #4:

I mean that currently, unless foo is already defined as a variable, both Octave and Matlab will parse "foo += bar" as a "command" and then attempt to find a function foo to call with the arguments '+=' and 'bar' (as strings). If we change Octave to recognize += as an operator in this case, then Octave's behavior will be different from Matlab in this case (and also for "foo +=bar").

Note that the way "commands" are parsed also depends on spacing. I thought it was documented in the Octave manual, but apparently not. Note that the following are handled differently (assuming the LHS is not already known to be a variable:

Yes, this is really unfortunate behavior.

Also my comment #2 should have said "NOT restarting your session", so foo.m is not parsed again. The only thing that changes is that a mycmd function is added to the path. In a way, this is a good thing, because it would be really bad for the behavior to also depend on what function names happen to be available on the path when a function is parsed.

John W. Eaton <jwe>
Project Administrator
Wed 13 Apr 2016 10:47:31 AM UTC, comment #3:

what do you mean by

will be handled differently in Matlab?

As far as I know "OP=" are not available at all in Matlab ...

Carlo de Falco <cdf>
Project Member
Tue 12 Apr 2016 05:21:16 PM UTC, comment #2:

The problem is that the parser is interpreting the expression as a command, equivalent to

You may have already discovered that if you evaluate example_1 separately before example_0 or if you replace the call to example_1 with the contents of that file, then you will not see the error. That's because a script file is parsed completely and then executed. So when parsing example_0 by itself, Octave doesn't know that Csb is a variable so it assumes it can be called as a function.

I suppose we can add a special case for OP= operators so that they may be parsed as expressions even if the LHS is not yet defined as a variable. However, this introduces a compatibility issue because then things like

will be handled differently in Matlab.

As I recall, checking to see if the symbol on the LHS is a function when parsing the expression is not the solution because the following is supposed to work:

Define in a file with mycmd undefined:

Then call it:

Now define

somewhere on the path (restarting your session) and call foo (1) again and it should display the argument to mycmd as a string.

John W. Eaton <jwe>
Project Administrator
Tue 12 Apr 2016 04:07:05 PM UTC, comment #1:

just an update noting that
the same error occurs if other
'OP=' opertaors are used.

Carlo de Falco <cdf>
Project Member
Tue 12 Apr 2016 01:05:52 PM UTC, original submission:

The attached files demonstrate an error in the interpreter
when applying the multiplication-assignment operator to a
variable defined in a script.

To reproduce the issue, try:

if the same commands contained in the script example_0
are issued directly on the command line, the problem does
not occur:

The same issue is also present in the satble release but the error message is less informative.

Carlo de Falco <cdf>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #36925:  test_comment_6.tgz added by cdf (300B - application/x-tar)
file #36921:  example_2.m added by lachlan (18B - d2l/unknowntype)
file #36908:  example_0.m added by cdf (38B - application/octet-stream)
file #36909:  example_1.m added by cdf (13B - application/octet-stream)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by cdf
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by cdf (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 13 Jun 2016 09:48:38 AM UTClachlanAttached File#37458=>Removed
    Mon 13 Jun 2016 09:48:17 AM UTClachlanAttached File-=>Added bug_47676_warn_cmd_operator.cset, #37459
    Mon 13 Jun 2016 04:40:04 AM UTClachlanAttached File-=>Added bug_47676_warn_cmd_operator.cset, #37458
      StatusNone=>Patch Submitted
      SummaryError applying multiplication-assignment operator to variable defined in script=>Cannot apply computed assignment to a variable defined after the code was parsed (e.g., in a script)
    Thu 12 May 2016 05:12:59 PM UTCrik5Dependencies-=>bugs #47909 is dependent
    Thu 14 Apr 2016 03:35:15 PM UTCcdfAttached File-=>Added test_comment_6.tgz, #36925
    Thu 14 Apr 2016 01:19:41 AM UTClachlanAttached File-=>Added example_2.m, #36921
      Severity3 - Normal=>4 - Important
    Wed 13 Apr 2016 01:32:32 PM UTCcdfCarbon-Copy-=>Added -unavailable-
    Tue 12 Apr 2016 01:05:52 PM UTCcdfAttached File-=>Added example_0.m, #36908
      Attached File-=>Added example_1.m, #36909

    Back to the top


    Powered by Savane 3.1-cleanup1