bugGNU Octave - Bugs: bug #65191, Refactor dynamic stack frame for...

 
 

bug #65191: Refactor dynamic stack frame for bytecode interpreter

Submitter:  Petter <petter>
Submitted:  Mon 22 Jan 2024 10:40:56 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 23 Jan 2024 04:04:26 PM UTC, comment #2: 

Made the changes in comment #1 and pushed here: https://hg.savannah.gnu.org/hgweb/octave/rev/2eabd2567846

(There are more style changes to make to the BCI branch over the next few months.)

Arun Giridhar <arungiridhar>
Group Member
Tue 23 Jan 2024 07:33:56 AM UTC, comment #1: 

I haven't looked at the functional differences of the patch yet. Just some initial comments (mostly style - sorry for nagging):

Please, include the bug number in the first line of the commit message.
Typo "unecessary" -> "unnecessary" in commit message.
Use two hash characters "##" if comments start at the beginning of a line in .m files.
Use two spaces and a hash character "  #" for inline comments in .m files.
Use one space after the negation operator. I was once told to read it as the English word "not" which would also be separated with a space from the following word.
Use empty assignment list for .m file functions that take no arguments (e.g., "nested2 ()").

Markus Mützel <mmuetzel>
Group administrator
Mon 22 Jan 2024 10:40:56 PM UTC, original submission:  

After discussions in the discourse thread about aligning the stack frame objects of the bytecode interpreter with how the non-bytecode frames work, I've done some much needed refactoring of the bytecode_fcn_stack_frame class.

The bytecode_fcn_stack_frame class was quite entangled with back and forth handling of scripts, nested, anonymous function etc, so I tried to make it cleaner by splitting it up and removing redundant code.


Refactor dynamic stack frame for bytecode interpreter

Split up the bytecode_fcn_stack_frame into fcn, script and nested
to detangle unessecary messiness.

Also solve some bugs where handles to nested function did not look
for symbols in the correct frame and did not push the closure frame
properly.

* call-stack.cc: Remove redundant check for bytecode frames
                 Use new stack_frame::create_xxx fcns
* stack-frame.cc: Refactor bytecode_fcn_stack_frame
* stack-frame.h: New create_bytecode_nested ()
                 New create_bytecode_anon ()
                 New create_bytecode_script ()
* ov_base.h: New enum value OCT_NESTED_HANDLE
* ov-fcn-handle.h: New get_closure_frame ()
* ov-fcn-handle.cc
* ov-ref.cc: Use offset overload
* ov.h: More friends
* pt-bytecode-vm.cc: Handle bytecode calls to nested handles
* pt-butecode-walk.cc: Remove fake offset workaround
* bytecode_nested.m: Update tests


Petter <petter>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55597:  33272.patch added by petter (121KiB - 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 arungiridhar (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by petter (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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-01-23 arungiridhar StatusNone Ready For Test
    2024-01-22 petter Attached File- Added 33272.patch, #55597

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code