Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function literals #3077

Merged
merged 24 commits into from Oct 23, 2019
Merged

Function literals #3077

merged 24 commits into from Oct 23, 2019

Conversation

@t-paul
Copy link
Member

t-paul commented Sep 16, 2019

This is the first step in implementing function literals.

ToDo:

  • Fix/Validate (lexical) scoping
  • Validate operator precedence and associativity
  • Disable function calls on $ variables
  • Check parser conflict, seems to be another dangling else case
  • Check warning/error cases
  • Recursive calls
  • Enable tail recursion elimination (by function name)
  • Add is_function
  • Add test cases
  • Mark as experimental feature

ToDo later...

  • Support for defining function in shortcut style
  • Support directly defining curried functions
  • Decide about behavior in customizer

grafik


hypot = function(x, y) sqrt(x * x + y * y);
mean  = function(x, y) (x + y) / 2;

func  = function(x) x == 0
    ? function(x, y) x + y
    : function(x, y) x - y;

module m(h, x, y) {
    echo(h = h(x, y));
    translate([h(x, y), 0, 0]) cube(1);
}

echo(func0 = func(0)(8, 3));
echo(func1 = func(1)(8, 3));

echo(l = let(f = function(x) x) sqrt(f(25)));

echo(f = (function(x) sqrt(x))(49));

color("red") m(hypot, 3, 4);
color("green") m(mean, 5, 8);
color("blue") m(function(x, y) sqrt(x + y), 5, 6);
@nophead

This comment has been minimized.

Copy link
Member

nophead commented Sep 16, 2019

Cool.

What does echo(hypot) do and what does m(1,2,3) do?

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 16, 2019

Currently those give:

echo(hypot);
ECHO: [function]
m(1,2,3);
WARNING: Ignoring unknown function 'h', in file debug.scad, line 10.
ECHO: h = undef
WARNING: Ignoring unknown function 'h', in file debug.scad, line 11.
WARNING: Unable to convert translate([undef, 0, 0]) parameter to a vec3 or vec2 of numbers, in file debug.scad, line 11
@nophead

This comment has been minimized.

Copy link
Member

nophead commented Sep 16, 2019

Makes sense.

So variables that have function literals assigned to them are normal variables with a new type.

What happens if you have a normal function with the same name?

Can they be recursive?

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 16, 2019

Hmm, good question. I have not checked yet what the current implementation does. But I guess we need to make sure it works.

I think normal functions will be shadowed if a variable with the same name exists.

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 16, 2019

Lexical scoping of variable captured in function definition

module scope(f) {
    a = 5;
    echo(f = f(a));
}

a = 3;
x = 9;
scope(function(x) echo(x = x, a = a) x + a);
// ECHO: x = 5, a = 3
// ECHO: f = 8
@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 16, 2019

Recursive function

module mod(modf) {
    echo(modf = modf(5));
}

func = function(x) x <= 0 ? 0 : x + func(x - 1);
echo(func = func(8));
mod(func);
ECHO: func = 36
ECHO: modf = 15
t-paul added 2 commits Sep 22, 2019
The central change is to move the dynamic scoping handling out of the
lifetime of the Context class. Context and all derived classes are now
handled via shared pointer so they can hang around inside a function
definition as closure representing the static scoping stack.
The dynamic part is moved into ContextHandle which is supposed to be
created and destroyed as the evaluation traverses the expression tree.
The ContextHandle object calls push()/pop() on it's associated Context
on construction/destruction.
@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 24, 2019

Functions returning another function, capturing a statically and resolving $a dynamically.

module m2(f1, f2) {
    a = 3;
    $a = 30;
    echo(f1 = f1(0.03), f2 = f2(0.07));
}

module m1(f) {
    a = 2;
    $a = 20;
    add2 = function(x) function(y) x + y + a + $a;
    m2(f, add2(0.2));
}

a = 1;
$a = 10;
add1 = function(x) function(y) x + y + a + $a;
m1(add1(0.1));
ECHO: f1 = 31.13, f2 = 32.27
@MichaelAtOz

This comment has been minimized.

Copy link
Member

MichaelAtOz commented Sep 24, 2019

I couldn't read that until my morning caffeine kicked in...

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 25, 2019

What about this one, slightly extending the Wikipedia examples for higher order functions? This covers both the takes one or more functions as arguments (twice) and returns a function as its result (add and mul) cases in one evaluation.

add3 = function(x) x + 3;
mul2 = function(x) x * 2;
twice = function(f, x) f(f(x));

add = function(x) function() twice(add3, x);
mul = function(x) function() twice(mul2, x);

echo(add = add(7)(), mul = mul(7)());
ECHO: add = 13, mul = 28
@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Sep 26, 2019

Hmm, looks like we want array slicing too to get a nice way to express fold:

a = [1, 2, 3, 4, 5, 6, 7, 8, 9];

fold = function(i, v, f) len(v) > 0 ? fold(f(i, v[0]), v[1:], f) : i;

sum = function(v) fold(0, v, function(x, y) x + y);
prd = function(v) fold(1, v, function(x, y) x * y);
cnt = function(v) fold(0, v, function(x, y) x + 1);
rev = function(v) fold([], v, function(x, y) concat(y, x));

echo(sum = sum(a), prd = prd(a), cnt = cnt(a), rev = rev(a));
// ECHO: sum = 45, prd = 362880, cnt = 9, rev = [9, 8, 7, 6, 5, 4, 3, 2, 1]

(NOTE: the array slicing code is not included here)

@kintel

This comment has been minimized.

Copy link
Member

kintel commented Oct 4, 2019

Some of the demos in https://github.com/openscad/list-comprehension-demos could probably be rewritten more elegantly using this feature

@kintel

This comment has been minimized.

Copy link
Member

kintel commented Oct 4, 2019

@nophead It looks like a literal function (in the variable namespace) shadows a classic function (in the function namespace), but only if the variable actually resolves to a function.
That's somewhat backwards compatible, but perhaps a bit surprising:

function some_func(t) = t*3;

module MyModule(some_func, x) {
    r = some_func(x);
    echo(r);
    sphere(r);
}

myfunc= function(t) t*2;
MyModule(0, 5);

Perhaps worth a warning?

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Oct 4, 2019

The question is also if we want to make the builtins available as "new style" functions (in the variable namespace). A similar effect would be a very limited support for namespaces (e.g. something like just the builtin namespace like ::sin()).

@nophead

This comment has been minimized.

Copy link
Member

nophead commented Oct 4, 2019

@kintel , is there a line missing from your example? I.e. you don't use myfunc.

I don't know what the warning would be. Parameter name is the same as a function name and is also used to pass a function literal. It might even be an error, rather than a warning as I don't think the result would be intended.

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Oct 4, 2019

Yes, the example is MyModule(0, 5); vs. MyModule(myfunc, 5);

@nophead

This comment has been minimized.

Copy link
Member

nophead commented Oct 4, 2019

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Oct 4, 2019

It would only interact with existing scripts in case of something like is_undef(sin). A variable with name sin would just shadow the builtin. I think.

Yes, it would be possible to pass builtins as anonymous function literal, but not calling it sin.

@kintel

This comment has been minimized.

Copy link
Member

kintel commented Oct 5, 2019

We can still populate the built-in top-level variable namespace with new-style functions, too give people the choice. That wouldn't break anything out of the box as classic functions are still available.
The namespace stuff would be cool, but perhaps better dealt with once we implement proper namespaces?

src/value.h Outdated Show resolved Hide resolved
@MichaelAtOz

This comment has been minimized.

Copy link
Member

MichaelAtOz commented Oct 8, 2019

Please reply to the emails for #3088. I copied Doug's last post there.

@kintel

This comment has been minimized.

Copy link
Member

kintel commented Oct 14, 2019

@t-paul How hard would it be to make this experimental, to give it some time to mature?

Also, should we open a ticket to address the performance regression against nightly?

@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Oct 14, 2019

I don't see a way to make the parser changes experimental. We could flag the actual function literal to be always be an undefined literal. Yes, a ticket to check how to get the performance back makes sense.

@kintel

This comment has been minimized.

Copy link
Member

kintel commented Oct 14, 2019

I think just making the usage of the function literal experimental is good enough, just to raise awareness that using this feature isn't 100% set in stone yet - makes it a lot easier to iterate.

@kintel
kintel approved these changes Oct 23, 2019
@t-paul

This comment has been minimized.

Copy link
Member Author

t-paul commented Oct 23, 2019

Awesome, lets go then. I'll rebase the Value rework stuff from @thehans (#3096) against master then as I guess the current one will auto-close with this branch gone.

@t-paul t-paul merged commit b3a5863 into master Oct 23, 2019
5 checks passed
5 checks passed
ci/circleci: openscad-appimage-64bit Your tests passed on CircleCI!
Details
ci/circleci: openscad-mxe-32bit Your tests passed on CircleCI!
Details
ci/circleci: openscad-mxe-64bit Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@t-paul t-paul deleted the function-literals branch Oct 23, 2019
@MichaelAtOz

This comment has been minimized.

Copy link
Member

MichaelAtOz commented Nov 26, 2019

Ronaldo has an issue on the Forum,

I can't see why the following generates a stack overflow:

    function move(a, g) =
      function(x) g(x+a) ;

    g1 = function(x) x;
    g2 = move(0,g1);
    g3 = move(0,g2);

    echo(g3_x=g3);
    echo(g3_2=g3(2));

If g3 is defined as g2 no error is reported.

With 2019.10.06 I get

Compiling design (CSG Tree generation)...
ECHO: g3_x = function(x) g((x + a))
ERROR: Recursion detected calling function 'g' in file fn literals.scad, line 4
TRACE: called by 'g', in file fn literals parkinbot.scad, line 4.
TRACE: called by 'g3', in file fn literals parkinbot.scad, line 13.
TRACE: called by 'echo', in file fn literals parkinbot.scad, line 13.
Compiling design (CSG Products generation)...

If I add an echo()

function move(a, g) =
  function(x)
    echo(a,g)
      g(x+a) ; 

I get infinite loop of echo in console, quiting GUI crashes.

I then did

function move(a, g) = echo(move=a,g)
  function(x) 
      echo(g_in_move=a,x)
        (x+a) ;


g1 = function(x)    
        
          x;
g2 = move(1,g1);
g3 = move(3,g2);

echo(g1_=g1);
echo(g1_1=g1(1));
echo(g2_=g2);
echo(g2_7=g2(7));
echo(g3_x=g3);
echo(g3_5=g3(5));

Which gets:

Compiling design (CSG Tree generation)...
ECHO: move = 1, function(x) x
ECHO: move = 3, function(x) echo(g_in_move = a, x) (x + a)
ECHO: g1_ = function(x) x
ECHO: g1_1 = 1
ECHO: g2_ = function(x) echo(g_in_move = a, x) (x + a)
ECHO: g_in_moveg = 1, 7
ECHO: g2_7 = 8
ECHO: g3_x = function(x) echo(g_in_move = a, x) (x + a)
ECHO: g_in_moveg = 3, 5
ECHO: g3_5 = 8
Compiling design (CSG Products generation)...
Geometries in cache: 0

The extra echo stops the infinite recursion, not sure why, I haven't had my morning coffee yet...

@MichaelAtOz

This comment has been minimized.

Copy link
Member

MichaelAtOz commented Nov 26, 2019

Oops, at line 4 of the second code, I dropped the g() in editing.
That reinstates the infinite recursion.

assert helps.

function move(a, g) = echo(move=a,g)
  function(m) 
      echo(m_in_move=a,m,g)
      assert(m<14)
        g(m+a) ;

g1 = function(x)
       echo(fx=x)
          x;
g2 = move(2,g1);
g3 = move(3,g2);

echo(g2_=g2);
echo(g3_x=g3);
echo(g3_5=g3(5));

Gives

Compiling design (CSG Tree generation)...
ECHO: move = 2, function(x) echo(fx = x) x
ECHO: move = 3, function(m) echo(m_in_move = a, m, g) assert((m 
ECHO: g2_ = function(m) echo(m_in_move = a, m, g) assert((m 
ECHO: g3_x = function(m) echo(m_in_move = a, m, g) assert((m 
ECHO: m_in_move = 3, 5, function(m) echo(m_in_move = a, m, g) assert((m 
**ECHO: m_in_move = 2, 8, function(x) echo(fx = x) x
ECHO: m_in_move = 2, 10, function(x) echo(fx = x) x
ECHO: m_in_move = 2, 12, function(x) echo(fx = x) x
ECHO: m_in_move = 2, 14, function(x) echo(fx = x) x
ERROR: Assertion '(m < 14)' failed in file fn literals ronaldo.scad, line 4
TRACE: called by 'g', in file fn literals ronaldo.scad, line 5.
TRACE: called by 'g3', in file fn literals ronaldo.scad, line 15.
TRACE: called by 'echo', in file fn literals ronaldo.scad, line 15.
Compiling design (CSG Products generation)...

My head hurts. Feels like a scope issue...
Also see the Forum, Ronaldo has other observations.

@MichaelAtOz

This comment has been minimized.

Copy link
Member

MichaelAtOz commented Nov 26, 2019

As echo(fx=x) is never called, g1 is never called, even tho g=g1[ function(x) echo(fx = x) x] as echo'd by 'm_in_move', the g that gets called is g2 again.

@kintel kintel added this to TODO in Test Dec 30, 2019
@kintel kintel moved this from TODO to No in Test Dec 30, 2019
@kintel kintel removed this from To Do in Patch release (2020.01 ?) Dec 30, 2019
@kintel kintel removed this from No in Test Dec 30, 2019
@kintel kintel added this to Don't Include in Patch release (2020.01 ?) Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.