Skip to content

Commit

Permalink
Fix nargin for scripts in bytecode interpreter (bug #65140)
Browse files Browse the repository at this point in the history
Make nargin calls in scripts properly match argv or number of function
arguments, instead of always zero.

* stack-frame.cc: Set nargin
* test/compile/bytecode.tst: Do new test
* test/compile/bytecode_script_nargin.m: New test file
* test/compile/module.mk: Add new file
  • Loading branch information
pvilhelm committed Jan 10, 2024
1 parent 680bc97 commit 3e80fa3
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
3 changes: 3 additions & 0 deletions libinterp/corefcn/stack-frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,9 @@ class bytecode_fcn_stack_frame : public stack_frame
auto eval_frame = access_link ();
auto parent_frame = static_link ();

// Set nargin to match the value of nargin in the eval frame
set_nargin (eval_frame->get_auto_fcn_var (stack_frame::NARGIN).int_value()); // TODO: Kinda wasteful fn calls

bool caller_is_eval_frame = eval_frame == parent_frame;
bool eval_frame_is_bytecode = eval_frame->is_bytecode_fcn_frame ();

Expand Down
27 changes: 24 additions & 3 deletions test/compile/bytecode.tst
Original file line number Diff line number Diff line change
Expand Up @@ -782,10 +782,31 @@
%! end
%! end
%!
%! %% Test that nargin is set properly
%!
%! __vm_enable__ (0, "local");
%! clear all
%! bytecode_script_nargin_call_recursive = true; % The script calls it self once if this symbol is true
%! bytecode_script_nargin_expected_value = nargin; % The expected value of nargin
%! bytecode_script_nargin;
%!
%! % Call in top scope, to check that Octave cli start options set nargin to whatever the user started Octave with
%! evalin ("base", "bytecode_script_nargin_call_recursive = true; bytecode_script_nargin_expected_value = nargin; bytecode_script_nargin;");
%!
%! % Call in function
%! eval ("function foo (a,b) bytecode_script_nargin_expected_value = nargin; bytecode_script_nargin; end; foo (1, 2);")
%!
%! __vm_enable__ (1, "local");
%! clear all
%! bytecode_script_nargin_call_recursive = true;
%! bytecode_script_nargin_expected_value = nargin;
%! bytecode_script_nargin;
%!
%! evalin ("base", "bytecode_script_nargin_call_recursive = true; bytecode_script_nargin_expected_value = nargin; bytecode_script_nargin;");
%! eval ("function foo (a,b) bytecode_script_nargin_expected_value = nargin; bytecode_script_nargin; end; foo (1, 2);")
%!
%! %% Cleanup after the tests
%! clear global bytecode_script_topscope_call_self
%! clear global bytecode_script_topscope_place
%! clear bytecode_script_topscope_cli_fn
%! clear all

## Test save() and load() from scripts.
%!testif ENABLE_BYTECODE_EVALUATOR
Expand Down
19 changes: 19 additions & 0 deletions test/compile/bytecode_script_nargin.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

s_whos = whos;

if length (s_whos)
s_whos = s_whos(end);

% Only in the top scope nargin is equal to the size of argv
if strcmp (s_whos.nesting.function, "top scope")
assert (length (argv ()) == nargin ())
end
end

assert (bytecode_script_nargin_expected_value == nargin)

if exist ("bytecode_script_nargin_call_recursive") && bytecode_script_nargin_call_recursive
bytecode_script_nargin_call_recursive = false;
bytecode_script_nargin;
bytecode_script_nargin_call_recursive = true;
end
1 change: 1 addition & 0 deletions test/compile/module.mk
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ compile_TEST_FILES = \
%reldir%/bytecode_range.m \
%reldir%/bytecode_return.m \
%reldir%/bytecode_scripts.m \
%reldir%/bytecode_script_nargin.m \
%reldir%/bytecode_script_topscope.m \
%reldir%/bytecode_script_topscope_assert.m \
%reldir%/bytecode_script_topscope_setup.m \
Expand Down

0 comments on commit 3e80fa3

Please sign in to comment.