bugGNU Octave - Bugs: bug #57229, embedded.cc example in...

 
 

bug #57229: embedded.cc example in examples/code starts an interactive interpreter

Submitter:  Carlo de Falco <cdf>
Submitted:  Thu 14 Nov 2019 03:33:48 AM UTC
   
 
Category:  Interpreter Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Release: 
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 15 Nov 2019 12:26:41 AM UTC, comment #7: 

Also works for me.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Thu 14 Nov 2019 11:11:54 PM UTC, comment #6: 

I pushed this on the default branch

https://hg.savannah.gnu.org/hgweb/octave/rev/de4bfeda8d9f

Tested and works ok for me.

Mike Miller <mtmiller>
Group Member
Thu 14 Nov 2019 10:22:58 PM UTC, comment #5: 

Mike, I think your change is OK as the default execute method.

I'm also working on some changes to make the interpreter class more flexible, for example by allowing a derived class to re-implement the execute and/or main_loop methods.  I think something like that is needed for the new way of implementing the GUI command window that I've been thinking about.  I'll try to not break things again.

John W. Eaton <jwe>
Group administrator
Thu 14 Nov 2019 09:23:42 PM UTC, comment #4: 

I'm testing the attached patch, which I think fixes the regression, and fixes a minor typo in the embedded.cc example. I think it would help to expand the comments a bit, to explain that the user can choose between interpreter::initialize and interpreter::execute and what the differences are, if any.

(file #47868)

Mike Miller <mtmiller>
Group Member
Thu 14 Nov 2019 09:04:05 PM UTC, comment #3: 

I bisected and this is the first bad commit causing this regression: https://hg.savannah.gnu.org/hgweb/octave/rev/7a9d2b0e6da8

The relevant change might be that interpreter::main_loop doesn't return immediately now when m_app_context is null, and maybe it used to be called by interpreter::execute but returned immediately.

Mike Miller <mtmiller>
Group Member
Thu 14 Nov 2019 07:52:48 PM UTC, comment #2: 

Adding jwe to the CC list since this is probably only resolvable with his help.

This might be quite simple.  It may be that the API has changed and it is the example code in embedded.cc that needs to change.

I can get this to work if I make this change


diff -r 7e6836784575 examples/code/embedded.cc
--- a/examples/code/embedded.cc        Thu Nov 14 11:14:25 2019 -0800
+++ b/examples/code/embedded.cc        Thu Nov 14 11:49:43 2019 -0800
@@ -39,7 +39,9 @@ main (void)

       // Tell the interpreter that we're ready to execute commands:

-      int status = interpreter.execute ();
+      interpreter.initialize ();
+      int status = 0;
+      //int status = interpreter.execute ();

       if (status != 0)
         {


It seems that the member function execute both calls initialize() and runs the interactive version.

If embedded.cc is going to change then the prototype for initialize() should probably also be changed to return an exit code rather than void.  After initialization, the code in embedded.cc is


if (status != 0)
  {
    std::cerr << "creating embedded Octave interpreter failed!"
              << std::endl;
    return status;
  }


which relies on a status indication output.

Rik <rik5>
Group administrator
Thu 14 Nov 2019 07:12:28 PM UTC, comment #1: 

Confirmed (on revision 2e988d51e401).

Mike Miller <mtmiller>
Group Member
Thu 14 Nov 2019 03:33:48 AM UTC, original submission:  

I am testing with hg_id = 7b15c8bded6f


$ /opt/octave/6.0.0/bin/mkoctfile --link-stand-alone embedded.cc
$ ./a.out
octave:1> version
ans = 6.0.0
octave:2> exit
GCD of [10, 15] is 5


while with previous versions


$ /opt/octave/4.4.0/bin/mkoctfile --link-stand-alone embedded.cc
guglielmo $ ./a.out
GCD of [10, 15] is 5


Carlo de Falco <cdf>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #47868:  fix-interp-execute.patch added by mtmiller (1KiB - 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 jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by cdf (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-11-15 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-11-14 mtmiller StatusPatch Submitted Ready For Test
    2019-11-14 mtmiller Attached File- Added fix-interp-execute.patch, #47868
        StatusConfirmed Patch Submitted
    2019-11-14 rik5 Carbon-Copy- Added jwe
    2019-11-14 mtmiller Severity3 - Normal 4 - Important
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code