Skip to content

Commit

Permalink
findvars on recursive custom functions doesn't include that function'…
Browse files Browse the repository at this point in the history
…s name

When finding unbound variables inside the definition of a custom JME
function, the scope didn't yet know about the function being defined, so
a recursive call of that function would produce the function's own name
as a free variable.

This changes makeJMEFunction to add the function to the scope that is
used when finding unbound variables inside its definition.
  • Loading branch information
christianp committed Dec 19, 2024
1 parent 6a98c8b commit 4598c79
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
4 changes: 3 additions & 1 deletion runtime/scripts/jme-variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
*/
makeJMEFunction: function(fn,scope) {
fn.tree = jme.compile(fn.definition,scope,true);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
const nscope = new jme.Scope([scope]);
nscope.addFunction(fn);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
var vars = external_vars.slice();
for(var i=0;i<tree.args.length;i++) {
Expand Down
4 changes: 3 additions & 1 deletion tests/jme-runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -19971,7 +19971,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
*/
makeJMEFunction: function(fn,scope) {
fn.tree = jme.compile(fn.definition,scope,true);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
const nscope = new jme.Scope([scope]);
nscope.addFunction(fn);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
var vars = external_vars.slice();
for(var i=0;i<tree.args.length;i++) {
Expand Down
12 changes: 12 additions & 0 deletions tests/jme/jme-tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ Numbas.queueScript('jme_tests',['qunit','jme','jme-rules','jme-display','jme-cal
deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('a -> a(x)')),['x'],'findvars on lambda where an argument is another lambda');
deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('[x,[y,z]] -> x+y+z+w')),['w'],'findvars on lambda with destructuring');
deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('undefined_function(x)')),['x', 'undefined_function'],'Undefined function names are presumed to be missing variables.');

const fn = Numbas.jme.variables.makeFunction({
parameters: [{type:'number', name:'x'}],
definition: 'if(x>0,0,f(x-1))',
name: 'f',
language: 'jme'
}, Numbas.jme.builtinScope);

const scope = new Numbas.jme.Scope([Numbas.jme.builtinScope]);
scope.addFunction(fn);
const vars = Numbas.jme.findvars(Numbas.jme.compile('f(2)'), scope);
assert.deepEqual(vars, [], "Recursive custom functions don't produce their own names in findvars");
});

QUnit.test('findvars in HTML',function(assert) {
Expand Down
4 changes: 3 additions & 1 deletion tests/numbas-runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -19307,7 +19307,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
*/
makeJMEFunction: function(fn,scope) {
fn.tree = jme.compile(fn.definition,scope,true);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
const nscope = new jme.Scope([scope]);
nscope.addFunction(fn);
var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
var vars = external_vars.slice();
for(var i=0;i<tree.args.length;i++) {
Expand Down

0 comments on commit 4598c79

Please sign in to comment.