bugGNU Octave - Bugs: bug #54465, Short Circuit Operators not...

 
 

bug #54465: Short Circuit Operators not working with negation operator

Submitted by:  Andrew Lofts <andrewlofts>
Submitted on:  Wed 08 Aug 2018 06:19:04 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  AndrewLofts Open/Closed:  Closed
Release:  4.4.1-rc2 Operating System:  GNU/Linux

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)

Tue 28 Aug 2018 11:42:52 PM UTC, comment #10:

I commented out two functions to suppress the compilation warnings in this cset (https://hg.savannah.gnu.org/hgweb/octave/rev/d0062829754a).

Rik <rik5>
Project Administrator
Tue 28 Aug 2018 08:36:20 PM UTC, comment #9:

Yeah, I eliminated the only call to that function. We can just comment it out for now.

John W. Eaton <jwe>
Project Administrator
Tue 28 Aug 2018 07:55:59 PM UTC, comment #8:

The change seems to work. I used

and all 52 tests pass.

I get this warning during compilation though:

Rik <rik5>
Project Administrator
Tue 28 Aug 2018 06:03:37 PM UTC, comment #7:

How about the following change?

http://hg.savannah.gnu.org/hgweb/octave/rev/8ac4bfa55053

With more work, maybe the optimization for the compound negation and elementwise logical operators could be preserved when the expression is not eligible for short circuiting.

John W. Eaton <jwe>
Project Administrator
Tue 28 Aug 2018 03:32:16 PM UTC, comment #6:

@jwe: This patch seems to have introduced a regression in performance which is documented at bug #45890.

Is there a way to be more clever about this so that certain compound expressions like

can be recognized as a single operation (pass multiply with appropriate flag to BLAS) rather than two operations (multiply done by BLAS, transpose done by Octave)?

Rik <rik5>
Project Administrator
Thu 09 Aug 2018 04:47:43 PM UTC, comment #5:

I see that jwe just pushed the patch for this to the stable branch. Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Wed 08 Aug 2018 08:46:36 PM UTC, comment #4:

The patch seems to work. Here are two tests

Also attached as a file. They should probably go in test/parser.tst and have a bug number associated.

(file #44743)

Rik <rik5>
Project Administrator
Wed 08 Aug 2018 08:23:03 PM UTC, comment #3:

I think the attached patch should fix the problem. But before committing, it needs a test and we should probably see whether it makes sense to try to preserve the compound binary operator optimization or simply remove it.

(file #44742)

John W. Eaton <jwe>
Project Administrator
Wed 08 Aug 2018 07:20:56 PM UTC, comment #2:

This problem appears to be some kind of incorrect handling of precedence. The evaluator is evaluating the "b == 2" expression first.

In any case, I recommend using || and && if you want short-circuit evaluation.

John W. Eaton <jwe>
Project Administrator
Wed 08 Aug 2018 07:01:54 PM UTC, comment #1:

Confirmed. Incidentally, version 4.2.1 is very old and no longer supported. You will have a better experience if you can update to the latest stable version. But, I confirmed this on the current development code as well.

Here is an even more concise example:

I used Octave's own negation operator just to make sure there wasn't something funny going on. I also removed the call to exist() and just use a constant. Octave's own short-circuiting operator works in this case, so this is an error in the parser because within if statements the '|' operator should map to '||' for logical statements.

Adding jwe to the CC list since he has spent the most time with this code.

(file #44741)

Rik <rik5>
Project Administrator
Wed 08 Aug 2018 06:19:04 PM UTC, original submission:

Hello, I am working on porting MATLAB to Octave, normally | and & will short circuit in if() statements just like MATLAB. However if a function with a negative is used like ~exist(),~strcmp() or ~isempty(), the statements do not seam to short circuit like MATLABs do. ~= and the functions without ~, however do work. Example: (Octave/Matlab)

Setting do_braindead_shortcircuit_evaluation doesn't seam to change anything.
Thank you for your help,
Andrew

Andrew Lofts <andrewlofts>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #44743:  tst_sc2.m added by rik5 (138B - text/x-matlab)
file #44742:  diffs.txt added by jwe (447B - text/plain)
file #44741:  tst_sc.m added by rik5 (127B - text/x-matlab)

 

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 rik5
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by andrewlofts (Submitted the item)
  • -email is unavailable- added by andrewlofts (Octave,Matlab ~SC)
  •  

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

    Only project members can vote.

     

     

     

    Follow 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-08-09 rik5 StatusPatch Reviewed => Fixed
        Open/ClosedOpen => Closed
        Releasedev => 4.4.1-rc2
    2018-08-08 rik5 Attached File- => Added tst_sc2.m, #44743
        CategoryNone => Interpreter
        StatusConfirmed => Patch Reviewed
    2018-08-08 jwe Attached File- => Added diffs.txt, #44742
    2018-08-08 rik5 Carbon-Copy- => Added jwe
    2018-08-08 rik5 Attached File- => Added tst_sc.m, #44741
        StatusNone => Confirmed
        Release4.2.1 => dev
        SummaryShort Circuit Operators not working for ~functions => Short Circuit Operators not working with negation operator
    2018-08-08 andrewlofts Carbon-Copy- => Added andrewlofts

    Back to the top


    Powered by Savane 3.3