patchGNU Octave - Patches: patch #8033, Implement missing function evalc()

 
 

patch #8033: Implement missing function evalc()

Submitter:  Julien Bect <jbect>
Submitted:  Sat 27 Apr 2013 08:41:40 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 15 Jan 2016 05:34:07 PM UTC, comment #18: 

Finally!  This function is complete and checked in here (http://hg.savannah.gnu.org/hgweb/octave/rev/5ed379c8decd).  I made a few changes which I'll document.

1) I added evalc to the list of new functions in the NEWS file.

2) Dropped const from nargin declaration.


-  const int nargin = args.length ();
+  int nargin = args.length ();


While it is true that nargin is const, it is almost never necessary to specify it.  "nargin" is a local variable and the compiler can determine whether the value is changed (it isn't) and can make appropriate optimizations.

3) Replace NULL with 0 for pointers


-  if (eval_exception != NULL)
+  if (eval_exception)


NULL is a macro which can expand to different values in the C language.  In C++, Bjarne Stroustrup himself recommends just using 0.  Eventually when Octave supports C++ 2011 the nullptr keyword could be used.  See this discussion on StackOverflow: http://stackoverflow.com/questions/3825668/c-c-checking-for-null-pointer.

4) Add semicolons to regular assert statements versus specialized %!assert.


 %!test
 %! [s, y] = evalc ("1");
-%! assert (s, "")
-%! assert (y, 1)
+%! assert (s, "");
+%! assert (y, 1);


The Octave convention is that '%!assert' does not end in a semicolon whereas the ordinary function call 'assert' does end with a semicolon because it is the same as any other piece of Octave code.

5) Use "local" option to warning to simplify code

The "local" option automatically restores a configuration variable or setting when the scope of the function disappears.  It is a convenient way to temporarily modify an option without having to write a long unwind_protect block.


 %!test
-%! quiet = warning ("query", "quiet");
-%! warning ("off", "quiet")
-%! unwind_protect
-%!   assert (evalc ("warning ('foo')"), "warning: foo\n")
-%! unwind_protect_cleanup
-%!   warning (quiet.state, quiet.identifier);
-%! end_unwind_protect
+%! warning ("off", "quiet", "local");
+%! assert (evalc ("warning ('foo')"), "warning: foo\n");



Rik <rik5>
Group administrator
Fri 15 Jan 2016 06:54:36 AM UTC, comment #17: 

The failing asserts are a side effect of _run_test_suite_ calling

warning ("on", "quiet");


Using plain test function works.

>> test libinterp/parse-tree/oct-parse.in.yy-tst
PASSES 21 out of 21 tests


I have made a new patch with better tests that consider the usage within “make check”.

(file #36042)

Oliver Heimlich <oheim>
Fri 15 Jan 2016 01:00:18 AM UTC, comment #16: 

I have produced a revised patch. A big novelty is that the Feval function is called internally, so I do not have to duplicate that code anymore.

However, I have problems with these failing unit tests:


>>>>> processing /home/oliver/Dokumente/octave/libinterp/parse-tree/oct-parse.in
.yy-tst
***** assert (evalc ("warning ('foo')"), "warning: foo\n")
!!!!! test failed
ASSERT errors for:  assert (evalc ("warning ('foo')"),"warning: foo\n")

  Location  |  Observed  |  Expected  |  Reason
     []                   warning: foo
   Strings don't match
shared variables   scalar structure containing the fields:
    x =  1
***** assert (evalc ("error ('foo')", "warning ('bar')"), "warning: bar\n")
!!!!! test failed
ASSERT errors for:  assert (evalc ("error ('foo')", "warning ('bar')"),"warning: bar\n")

  Location  |  Observed  |  Expected  |  Reason
     []                   warning: bar
   Strings don't match
shared variables   scalar structure containing the fields:
    x =  1


Both assert statement work in the command window. Any ideas?

(file #36038)

Oliver Heimlich <oheim>
Thu 14 Jan 2016 02:59:53 PM UTC, comment #15: 

Can someone update this patch to take advantage of the new execption based error handling?  After that it should really be brought in to Octave core.  The patch has been mostly satisfactory, but has stayed here on the patch tracker for 3 years.

Rik <rik5>
Group administrator
Fri 27 Nov 2015 06:31:35 PM UTC, comment #14: 

@gyom, I didn't look into the error_state changes yet. Feel free to update the patch.

Oliver Heimlich <oheim>
Fri 27 Nov 2015 06:06:52 PM UTC, comment #13: 

@oheim: Did you have time to adjust your patch following Rik's suggestion? Otherwise I might try to have a look. Thanks!

Guillaume <gyom>
Mon 12 Oct 2015 04:39:58 PM UTC, comment #12: 

@Oliver: The patch was ready for the development branch, but now the core has switched to using try/catch blocks and exceptions.  Could you refresh your patch to use essentially the same style as eval in oct-parse.cc.  It looks pretty simple to do that.  Then I think we should commit this before there is any more drift in the code base.

Rik <rik5>
Group administrator
Fri 17 Jul 2015 11:08:02 AM UTC, comment #11: 

What's the status of this patch? It seems to work fine for all of us who tried it, and it would be a great addition to Octave. Thanks!

Guillaume <gyom>
Mon 29 Jun 2015 10:26:56 AM UTC, comment #10: 

matlab2014b on Linux also captures the output of ls in the return value of evalc('system(''ls'')');

It is far better to have an evalc implementation that differs from matlab with regard to the output of system than to not have evalc at all.

I applied the patch and and it works for me.

Anonymous
Thu 21 May 2015 12:50:30 PM UTC, comment #9: 

Sorry, I hadn't noticed it was mentioned the function's documentation. It is perhaps not so much of an issue as 'system' has its own mechanism to capture output if needed.

Thanks for this patch!

Guillaume <gyom>
Wed 20 May 2015 04:44:01 PM UTC, comment #8: 

True, and this is also documented in the function's documentation string. I don't know if MATLAB can handle this.

Oliver Heimlich <oheim>
Wed 20 May 2015 11:14:33 AM UTC, comment #7: 

I have just tried the patch - it would be great to have evalc available in Octave.

While it works with the test examples, it does not seem to handle this sort of call:


evalc('system(''ls'')');


Guillaume <gyom>
Sat 16 May 2015 10:15:56 PM UTC, comment #6: 

Please find a revised patch as a new attachement. The new version also handles warnings (on stderr) and supports multiple output arguments like eval does. The function should therefore be MATLAB compatible, but I cannot verify that.

I have also implemented some unit tests, updated the manual, copyright statements and @seealso references for related functions.

(file #34027)

Oliver Heimlich <oheim>
Sat 16 May 2015 09:17:14 AM UTC, comment #5: 

Thanks for your remarks. The function is currently being thoroughly tested as part of the upcoming doctest package.

I will prepare a patch for oct-parse.in.yy with your remarks and any improvements which are going to be found in the tests.

I don't understand the code for unwind_protect and buffer_error_messages in the functions eval and evalin. Is that explained somewhere?

Oliver Heimlich <oheim>
Sat 16 May 2015 04:20:23 AM UTC, comment #4: 

I'll have to let jwe do the final review.

The function works for me, however we would bring it into core so it would use the DEFUN rather than DEFUN_DLD macro.  The texinfo block would also need to be formatted, as it is for other core functions, as just a single string.  This requires using a '\' to do line continuation.  Otherwise, very short and sweet.


Rik <rik5>
Group administrator
Thu 14 May 2015 06:25:46 PM UTC, comment #3: 

Please find attached a new evalc implementation that does not require changes to the pager class and could already be used within an oct-file.

(file #34015)

Oliver Heimlich <oheim>
Sun 28 Apr 2013 05:56:41 PM UTC, comment #2: 

You have an unwind_protect frame in Fevalc to protect the buffer_error_messages variable.  Shouldn't you also use it to protect the pager_stream evalc mode?

Fevalc needs a doc string, and needs to be added to the manual.

Is there some more general concept that we can capture in the pager instead of just "evalc mode"?

Does this approach for implementing evalc work if the pager is not enabled?  What about for output that doesn't go through the pager at all?  For example, what happens if someone uses "system" to run a program inside a .m file, or a DEFUN function that writes directly to std::cout?

John W. Eaton <jwe>
Group administrator
Sat 27 Apr 2013 08:42:37 AM UTC, comment #1: 
Julien Bect <jbect>
Sat 27 Apr 2013 08:41:40 AM UTC, original submission:  

The attached patch implements the missing function evalc().

It is a preliminary version (documentation missing) but it is already mature enough to be tested.

Julien Bect <jbect>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36038:  evalc-revised.patch added by oheim (5KiB - text/x-patch)
file #34027:  evalc.patch added by oheim (6KiB - text/x-patch)
file #34015:  evalc.cc added by oheim (2KiB - text/x-c)
file #27957:  jbPatchEvalc4.patch added by jbect (7KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gyom (Posted a comment)
  • -email is unavailable- added by oheim (Updated the item)
  • -email is unavailable- added by cbm
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by jbect (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 logged-in users can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-01-15 rik5 StatusIn Progress Done
        Open/ClosedOpen Closed
    2016-01-15 oheim Attached File- Added evalc-revised-passing-tests.patch, #36042
    2016-01-15 oheim Attached File- Added evalc-revised.patch, #36038
    2015-10-12 rik5 StatusNeed Info In Progress
    2015-05-16 oheim Attached File- Added evalc.patch, #34027
    2015-05-14 oheim Attached File- Added evalc.cc, #34015
    2015-02-10 cbm Carbon-Copy- Added -email is unavailable-
    2013-08-08 rik5 StatusNone Need Info
    2013-04-27 jbect Attached File- Added jbPatchEvalc4.patch, #27957

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code