bugGNU Octave - Bugs: bug #62552, In-place operator spacing works...

 
 

bug #62552: In-place operator spacing works for 'i' and 'j', but not other variable names

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Tue 31 May 2022 08:57:31 PM UTC
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Missed Error or Warning
Status:  In Progress Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * 7.1.0 Operating System:  * Any
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Sun 05 Jun 2022 11:53:00 AM UTC, comment #37: 

I pushed jwe's change to the wording in the NEWS file here:
https://hg.savannah.gnu.org/hgweb/octave/rev/f03902a39810

And pushed the change to the wording of the error message here:
https://hg.savannah.gnu.org/hgweb/octave/rev/277e31f0bb60

IIUC, the last remaining point is the changes in the manual that jwe proposed in comment #22

Markus Mützel <mmuetzel>
Project Member
Sat 04 Jun 2022 11:54:42 PM UTC, comment #36: 

yes, I think that helps quite a bit. if nothing else it provides enough of a hint as to the cause of the problem to guide someone to the solution. Thanks.

Nicholas Jankowski <nrjank>
Project Member
Sat 04 Jun 2022 02:17:11 PM UTC, comment #35: 

I'm attaching a changeset for the error message in these cases.
With it, I see:

octave:1> a = 2
a = 2
octave:2> a test
error: variable "a" used as function in command style expression
octave:3> a +2
error: variable "a" used as function in command style expression
Check whitespace around potential binary operator.
octave:4>

Are these messages more helpful?

(file #53273)

Markus Mützel <mmuetzel>
Project Member
Fri 03 Jun 2022 05:51:22 PM UTC, comment #34: 

Definitely a good explanation for anyone encountering the behavior.

Arun Giridhar <arungiridhar>
Project Member
Fri 03 Jun 2022 04:28:49 PM UTC, comment #33: 

Personally, I appreciate the detailed description to something that  is a backward compatability change.  I also think a change to something like "variable %s used in command-style function call" wolud be an improvement since it shouldn't require any logic changes.

Nicholas Jankowski <nrjank>
Project Member
Fri 03 Jun 2022 03:30:25 PM UTC, comment #32: 

I might be biased because I read the remainder of this bug report before reading your NEWS entry. But imho, it describes pretty clearly what is going on, why the behavior changed, and what a user can do to fix their code. (That last part is only implicitly covered. But that's sufficient imho.)

Markus Mützel <mmuetzel>
Project Member
Fri 03 Jun 2022 02:11:58 PM UTC, comment #31: 

Yeah, it is long.

Thanks for catching the edit error.

Do you think what I wrote helps with understanding the issues?  It seems there is a lot of confusion about this feature.

It appears that where Octave is doing

if (is_var && is_word_list_cmd ())
  error ("%s used as variable and later as function", nm.c_str ());

Matlab is now doing something like

if (is_word_list_cmd ())
  {
     // Search for function only since this appears to be a command-style function call.  If a function is found, call it with remaining arguments.  If not found, error with "unrecognized function or variable" message.
  }

That explains their error message.

John W. Eaton <jwe>
Project Administrator
Fri 03 Jun 2022 01:55:18 PM UTC, comment #30: 

That's pretty long compared to the other paragraphs in the NEWS file. But, I guess we need to be lengthy if we want to describe it completely and correctly.

Minor change:

-expression like `a + b +c` will be not be parsed as a command-style
+expression like `a + b +c` will not be parsed as a command-style

As for the behavior of Matlab R2022a:

>> clear all
>> nonexistent_fcn = 1

nonexistent_fcn =

     1

>> nonexistent_fcn -1
Unrecognized function or variable 'nonexistent_fcn'.

>> ls = 1

ls =

     1

>> ls -1
'-1' not found.

Markus Mützel <mmuetzel>
Project Member
Fri 03 Jun 2022 01:49:04 PM UTC, comment #29: 

Here is my attempt.  I don't know how to make it shorter and keep it clear without having to inconvenience users by referring them to the manual.

- Parsing of command-style function calls has changed to improve
consistency of behavior and compatibility with Matlab.  This change
affects statements that begin with binary operator expressions when the
first operand is a plain symbol followed by a whitespace character and
the binary operator is not followed by a whitespace character.  For
example, the statement `cmd -option` is parsed as a command-style
function call, not binary subtraction operation.

    This change affects all binary operators: `+`, `-`, `*`, `/`, `\`,
`^`, `.*`, `./`, `.\`, `.^`, `|`, `&`, `||`, `&&`, `+=`, `-=`, `*=`,
`/=`, `\=`, `^=`, `.*=`, `./=`, `.\=`, `.^=`, `|=`, and `&=`.

    Previous versions of Octave would attempt to determine whether the first
operand in an expression was a variable, and if so, parse expressions
like `var -val` as an expression.  However, this attempt to "do the
right thing" could cause trouble (for example, if a variable is only defined
conditionally).

    Now, parsing command-style function calls is purely based on syntax.

    Command-style function call syntax is only allowed at the beginning
of a statement.  Expressions in other contexts are not affected, so an
expression like `a + b +c` will be not be parsed as a command-style
function call.

    For compatibility with Matlab, a binary expression at the beginning
 of a statement that starts with one of the symbols `I`, `i`, `J`, `j`,
 `Inf`, `inf`, `NaN`, `nan`, or `pi` is never parsed as a command-style
 function call.  Octave also extends this behavior to `e`, which is not
 present as a special numeric constant in Matlab.

    Note that full compatibility with Matlab, which does not have the
OP= operators, would require that even `a -= b` is parsed as a
command-style function call.  As that would significantly hinder the use
of these operators, Octave treats them as the other binary operators
instead of providing fully compatible behavior.

But now I'm wondering (again!) whether we have the behavior completely correct.  Could someone check what a recent version of Matlab does for the following?

clear all
nonexistent_fcn = 1
nonexistent_fcn -1
ls = 1
ls -1

John W. Eaton <jwe>
Project Administrator
Fri 03 Jun 2022 06:54:12 AM UTC, comment #28: 

I guess the word I was looking for is "ambiguously".

I pushed the updated note here:
https://hg.savannah.gnu.org/hgweb/octave/rev/505ec2f4e16f

Markus Mützel <mmuetzel>
Project Member
Fri 03 Jun 2022 06:10:45 AM UTC, comment #27: 

Another stab at the note in the NEWS file:

- When an expression involving operators could be interpreted arbitrarily
either as command style syntax or function style syntax, it is
interpreted as command style syntax in more cases than before.  To still
be interpreted as function style syntax, inplace operators (`+=`, `-=`,
`*=`, `.*=`, `/=`, `./=`, `\=`, `.\=`, `^=`, `.^=`, `|=`, `&=`) must now
either be followed by a whitespace character or must not be preceded by a
whitespace character.  For expressions with binary operators (`+`, `-`,
`*`, `.*`, `/`, `./`, `\`, `.\`, `^`, `.^`, `|`, `&`, `||`, `&&`), the
same rules apply.
E.g., `a + b`, `a+ b`, or `a+b` are valid expressions if `a` is a
variable.  In contrast, `a +b` will throw an error if `a` is a variable.
The latter example is now interpreted as a command syntax expression
equivalent to the function syntax expression `a ("+b")`.

I'm not sure about the usage of the word "arbitrarily" in the first sentence. I'd like to express that - at least theoretically - these expressions could be either command style or function style. But the "meaning" in either of these interpretations are different.
Is there a better English word for that?

Markus Mützel <mmuetzel>
Project Member
Fri 03 Jun 2022 04:08:03 AM UTC, comment #26: 

The code in pt-idx.cc where the error message occurs is

if (is_var && is_word_list_cmd ())
  error ("%s used as variable and later as function", nm.c_str ());

This code is executed while evaluating the index expression, not when parsing it.  Since we know that it is a word-list command, then yes, we could change the error to "variable %s used in command-style function call".

IF we check to see whether the initial characters of the first argument in the word-list command match a binary operator, then we could issue a warning like you suggest: "spacing around %s operator makes variable %s appear to be used in command-style function call".

John W. Eaton <jwe>
Project Administrator
Fri 03 Jun 2022 02:43:31 AM UTC, comment #25: 

I think that's better, but it can probably be more direct rather thna 'Hint:'. 

If this error is only triggerd by command stlye function calls, then the error message should explicitly say something to that effect, and maybe say something about the spacing being the cause.

'variable %s used in command-style function call"

if ambiguous operator spacing can be identified during the error, then separate the comment #20 'a test' error from the 'a +1' or 'a +=1' errors.

'ambiguous operator spacing makes variable %s appear to be a used in a command-style function call'

i'm not sure what is best, just that before I realized any function can be called command style, when I moved from octave 6.4 to 7.1 and started seeing this error frequently, I really had  no idea why the operations were triggering that error and the message didn't really help.

Nicholas Jankowski <nrjank>
Project Member
Fri 03 Jun 2022 01:48:31 AM UTC, comment #24: 

@nrjank: is comment #7 useful to give a better error?

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 04:50:38 PM UTC, comment #23: 

Yeah. I was thinking about the same. Also chained assignments like
`c = a +=b` (which is still ok) aren't really covered by that NEWS entry. But I thought that it was "good enough"™ to raise awareness.
Please, exchange it with something more precise if possible.

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 04:44:47 PM UTC, comment #22: 

The wording in the news file is still not quite right.  We shouldn't say anything about assignment here because that's not the issue.  It's all about how command-style function calls are parsed.  For example,

a + b -c

is not a problem.  This expression doesn't look like a command-style function call because there is a space after the first operator.  It would be a command-style function call if it were written like this:

a +b - c

I can take a shot at the news entry.  Maybe it's best to also reference the "Command Syntax and Function Syntax" section in the manual.  That section also needs some examples an explanation of what happens with operators, not just word-list commands.  And the section describing binary operators should probably also link to the command/function call syntax section.

Note that command-style function call parsing is entirely syntax based.  In earlier versions of Octave (and I thought Matlab) the parser tried to determine whether the first word in the statement was a variable and if it was, didn't accept things like "a +b" as command-style function calls.  It no longer does that.

Also, if we were completely compatible with Matlab, then even

foo += bar

would be parsed as a command-style function call because there is no "+=" operator in Matlab and this is a case of a binary operator not followed by a space.  But if we followed that rule, then the OP= operators would be almost completely worthless.

FWIW, I've never liked the way command-style function calls are parsed, but we have to do it for compatibility with Matlab.

John W. Eaton <jwe>
Project Administrator
Thu 02 Jun 2022 04:31:32 PM UTC, comment #21: 

Ok. Are all cases specific to command style calling? I would think any function style call attempt of a variable would always be taken as a subsref or subsasgn call and trigger errors of that variety.

Was just thinking of whether there would be a more instructive message but wouldn’t want to suggest something that makes it incorrect for other uses.

Nicholas Jankowski <nrjank>
Project Member
Thu 02 Jun 2022 04:24:48 PM UTC, comment #20: 

Another example to trigger that error:

a = 1;
a test

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 04:21:05 PM UTC, comment #19: 

Is there any separate interest in modifying the error message to better understand how to avoid the error?

I’ve only ever seen that particular “used as variable and later as function” error in this particular use case. What other cases are expected to set it off, and can this particular case (spacing induced ambiguity) be caught with a more specific message?”

Nicholas Jankowski <nrjank>
Project Member
Thu 02 Jun 2022 12:58:38 PM UTC, comment #18: 

Looks good!

I'll close this as Won't Fix since the spacing behavior and also the exceptions (i, j, I, J, etc) are all consciously done for Matlab compatibility. The note in NEWS should be enough going forward.

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 12:50:53 PM UTC, comment #17: 

I sneaked a "without assignment" in.
Imho, together with the example that is probably good enough for the NEWS file. I pushed the change (with some minor modifications) here:
https://hg.savannah.gnu.org/hgweb/octave/rev/201e29541ef5

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 12:45:32 PM UTC, comment #16: 

Yes, that should work. Though that only applies to bare statements and not assignment expressions as you said, mentioning that might complicate it.

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 12:39:49 PM UTC, comment #15: 

Maybe this?

- For compatibility with command syntax, inplace operators (`+=`, `-=`,
`*=`, `.*=`, `/=`, `./=`, `\=`, `.\=`, `^=`, `.^=`, `|=`, `&=`) must now
either be followed by a whitespace character or must not be preceded by a
whitespace character.  For commands with binary operators (`+`, `-`, `*`,
 `.*`, `/`, `./`, `\`, `.\`, `^`, `.^`, `|`, `&`, `||`, `&&`) without
assignment, the same rules apply.  That was not a requirement in previous
versions of Octave.
E.g., `a + b`, `a+ b`, or `a+b` are valid if `a` is a variable.  In
contrast, `a +b` is not valid if `a` is a variable.  The latter example
is now interpreted as command syntax equivalent to the function syntax
`a ("+b")`.

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 12:34:11 PM UTC, comment #14: 

Maybe an example after the boolean rules about space? Like

Therefore `a + b` is valid, as are `a+ b` and `a+b` but `a +b` is interpreted as a command `a` with argument `+b` and could cause an error. This is done for Matlab compatibility.

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 12:29:30 PM UTC, comment #13: 

That's still not quite it. Binary operators are only affected if the command is a "bare" binary operations. Something like this is still ok:

>> a = 1; b = 2;
>> c = a +b
c = 3

But `a +b` (without the assignment) is not.

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 12:23:14 PM UTC, comment #12: 

Oops. You are right. Also removing the whitespace before the operator also avoids the issue.
Is the following change better?

diff -r 28d63e8bcaa7 etc/NEWS.7.md
--- a/etc/NEWS.7.md        Thu Jun 02 11:07:54 2022 +0200
+++ b/etc/NEWS.7.md        Thu Jun 02 14:21:05 2022 +0200
@@ -87,9 +87,11 @@
 is no longer the case.

 - For compatibility with command syntax, inplace operators (`+=`, `-=`,
-`*=`, `.*=`, `/=`, `./=`, `\=`, `.\=`, `^=`, `.^=`, `|=`, `&=`) must now
-be followed by a whitespace character.  That was not a requirement in
-previous versions of Octave.
+`*=`, `.*=`, `/=`, `./=`, `\=`, `.\=`, `^=`, `.^=`, `|=`, `&=`) and
+binary operators (`+`, `-`, `*`, `.*`, `/`, `./`, `\`, `.\`, `^`, `.^`,
+`|`, `&`, `||`, `&&`) must now either be followed by a whitespace
+character or must not be preceeded by a whitespace character.  That was
+not a requirement in previous versions of Octave.

 - The `mldivide` function (i.e., the `\` operator) now uses an LU
 decomposition to solve nearly singular full square matrices.  This is

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 12:15:39 PM UTC, comment #11: 

@Markus: Should the note say all binary operators instead of only the in-place ones? I get the same error for binary operators with the test in comment #3.

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 09:13:43 AM UTC, comment #10: 

I added a note about that change to the NEWS file here:
https://hg.savannah.gnu.org/hgweb/octave/rev/28d63e8bcaa7

(Unfortunately, I used the wrong bug number in the commit message... 😓)

Markus Mützel <mmuetzel>
Project Member
Thu 02 Jun 2022 01:37:41 AM UTC, comment #9: 

Agreed. I mentioned that bug report and discussion in comment #1.

The spacing behavior itself is probably Won't Fix for Matlab compatibility, but the message to the user might be discussed, per nrjank's comment.

Arun Giridhar <arungiridhar>
Project Member
Thu 02 Jun 2022 12:41:00 AM UTC, comment #8: 

See also bug #60882 where the changes in parsing command-style functions were discussed.

John W. Eaton <jwe>
Project Administrator
Wed 01 Jun 2022 06:51:36 PM UTC, comment #7: 

This cosmetic change might help guide new users. Thoughts?

diff -r d5997bbdb641 libinterp/parse-tree/pt-idx.cc
--- a/libinterp/parse-tree/pt-idx.cc    Tue May 31 23:34:23 2022 -0400
+++ b/libinterp/parse-tree/pt-idx.cc    Wed Jun 01 14:41:18 2022 -0400
@@ -366,7 +366,7 @@ namespace octave
         std::string nm =  id->name ();

         if (is_var && is_word_list_cmd ())
-          error ("%s used as variable and later as function", nm.c_str ());
+          error ("%s used as variable and later as function. Hint: inconsistent spacing around operators?", nm.c_str ());

         if (! is_var)
           {

Arun Giridhar <arungiridhar>
Project Member
Wed 01 Jun 2022 05:41:25 PM UTC, comment #6: 

matlab 2022a doesn't recognze ++ and += (the error correction actually suggests b++ --> b = b+1 and b+=c --> b = b + c), looking at the behavior with other operators:

>> b = 1
b =
     1
>> b + 1
ans =
     2
>> b+ 1
ans =
     2
>> b +1
Unrecognized function or variable 'b'.

>> i + 1
ans =
   1.0000 + 1.0000i
>> i+ 1
ans =
   1.0000 + 1.0000i
>> i +1
ans =
   1.0000 + 1.0000i
>> i = 1
i =
     1
>> i + 1
ans =
     2
>> i+ 1
ans =
     2
>> i +1
ans =
     2

So, Octave's behavior does seem to be matlab compatible.  I personally think both error messages are lacking. Matlab's is worse than Octave's, as I've just defined a variable and it's telling me "Unrecognized function or variable", when the reality is it's looking for a function and not finding one.

Octave's message "k used as variable and later as function" is also confusing, as it's likely not clear to anyone not familiar with the command vs function syntax why the missing space matters when operator whitespace doesn't matter in so many other contexts, and when it's a new set of error occurrences in Octave 7 (I get no errors on any of these statements in v6.4.0).

Nicholas Jankowski <nrjank>
Project Member
Wed 01 Jun 2022 05:09:53 PM UTC, comment #5: 

Since this behavior is there to accommodate specific Matlab behavior, should it be closed as Won't Fix?

Arun Giridhar <arungiridhar>
Project Member
Wed 01 Jun 2022 03:38:56 AM UTC, comment #4: 

The function base_lexer::handle_identifier in libinterp/parse-tree/lex.ll includes the following code:

    // For compatibility with Matlab, the following symbols are
    // handled specially so that things like
    //
    //   pi +1
    //
    // are parsed as an addition expression instead of as a command-style
    // function call with the argument "+1".  Also for compatibility with
    // Matlab, if we are at the top level workspace, do not consider IDENT
    // as a possible command if it is already known to be a variable.

    if (m_at_beginning_of_statement
        && ! (m_parsing_anon_fcn_body
              || ident == "e" || ident == "pi"
              || ident == "I" || ident == "i"
              || ident == "J" || ident == "j"
              || ident == "Inf" || ident == "inf"
              || ident == "NaN" || ident == "nan"))
      tok->mark_may_be_command ();

The comment is wrong about a difference in behavior in the top level workspace.  I changed the code here: http://hg.savannah.gnu.org/hgweb/octave/rev/ef865fcd4611 but forgot to update the comment.  I fixed it here: http://hg.savannah.gnu.org/hgweb/octave/rev/1589a7967d1e

Does Matlab still behave this way, at least for the functions like i, j, pi, inf, Inf, nan, and NaN that it has?  I know that some of these (e, I, J) are specific to Octave.  But it seems best to me that they behave in the same way.  If there is disagreement about that, then maybe we should deprecate the Octave-specific names and eventually remove them?

John W. Eaton <jwe>
Project Administrator
Wed 01 Jun 2022 01:26:37 AM UTC, comment #3: 

It's not restricted to the increment operators though. The same behavior happens for practically any binary operator. https://octave.discourse.group/t/strange-behaviour-of-operator/2755/3

Test:

i = k = 0;
i +k
k +i

Arun Giridhar <arungiridhar>
Project Member
Wed 01 Jun 2022 12:30:52 AM UTC, comment #2: 

To make it even clearer, the normal failing case is

k = 0; k +=1

which issues an error because this looks like command function calling of the function 'k' with string argument '+=1'.

The equivalent

i = 0; i +=1

does not issue an error and instead calls the in-place addition operator.

This might have something to do with i,j being complex constants.  Incidentally, this is also an issue for 'I', 'J' which are also parsed as complex constants.  In addition, 'i', 'j', 'I', 'J' are also functions and that may be a further complication.

Rik <rik5>
Project Administrator
Tue 31 May 2022 09:32:48 PM UTC, comment #1: 

Based on bug #60882, it was indeed a conscious decision to apply that spacing rule to all binary operators. So this bug is only about why it does not trigger the error for i and j. Are they being parsed as complex constants instead of variables?

Arun Giridhar <arungiridhar>
Project Member
Tue 31 May 2022 08:57:31 PM UTC, original submission:  

This is based on a user report at https://octave.discourse.group/t/strange-behaviour-of-operator/2755

The ++ and -- operators need to hug their operands, but does that rule extend to the += and friends as well? If yes, then i and j are not triggering that error. If not, then variables other than i and j need to be allowed that spacing.

Calling this a missed error since i and j do not trigger the spacing error, but please reclassify it if that is not right.

octave:3> i = 0; i += 1; i+=1; i +=1; i+= 1;
octave:4> j = 0; j += 1; j+=1; j +=1; j+= 1;
octave:5> k = 0; k += 1; k+=1; k +=1; k+= 1;
error: k used as variable and later as function

octave:7> k +=1
error: k used as variable and later as function

octave:8> i +=1
i = 5

octave:9> j +=1
j = 5

Arun Giridhar <arungiridhar>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by arungiridhar (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 project members can vote.

     

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-06-04 mmuetzel Attached File- Added bug62552-var-as-fcn-error-msg.patch, #53273
    2022-06-03 rik5 Carbon-CopyRemoved 72865 -
    2022-06-02 mmuetzel Open/ClosedClosed Open
    2022-06-02 mmuetzel StatusWont Fix In Progress
    2022-06-02 arungiridhar StatusConfirmed Wont Fix
        Open/ClosedOpen Closed
    2022-06-01 rik5 StatusNone Confirmed
        SummaryOperator spacing works for i and j but no other variables In-place operator spacing works for 'i' and 'j', but not other variable names

    Back to the top


    Powered by Savane 3.9