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

Crash with function fields and inherited classes #351

Open
distantforest1 opened this issue Jan 20, 2025 · 3 comments
Open

Crash with function fields and inherited classes #351

distantforest1 opened this issue Jan 20, 2025 · 3 comments

Comments

@distantforest1
Copy link

distantforest1 commented Jan 20, 2025

Hi I'm experiencing a crash when writing some code involving function fields.
It gives me this message.
Please send ...\lobster.exe.exp.log to the developer! --runtime-stack-traces samples\bugs\field_function_superclass_crash.lobster

It appears to happen when using super-classes and fields which are functions.

I've created a minimal reproduction version of the issue I'm having here:

import std

def void_fn() -> void

class ancestor:
  name = "bob"

class person : ancestor // removing `ancestor` as the super-class stops the crash
  on_talk: void_fn = fn(): pass()
  finished = false

  def talk():
    let talk = on_talk
    talk()

class group:
  people = []::person

  def add_person():
    let p = person{}
    p.on_talk = fn():
      print("hello world")
      p.finished = true // removing this also stops the crash

    people.push(p)

  def update_person():
    for(people) p:
      p.talk()

    people = people.filter(): _p.finished == false

let group = group{}

group.add_person // Removing this also stops the crash
group.add_person
group.update_person

Here is the above code as a lobster file:
function_field_crash.lobster
run in the fashion of:
bin\lobster.exe samples\bugs\function_field_crash.lobster

And the crash log:
lobster.exe.exp.log

Compiled on Windows 10 home. Using the latest master.

@aardappel
Copy link
Owner

Thanks for reporting!

The problem is that Lobster function values don't encapsulate free variables (such as p in this example), they are not "closures". They can only safely be executed in a context where that free variable still exists, which is something that is statically checked if you don't use explicit function types. But with explicit function types, that information is lost and it doesn't detect that this is a problem.

For example, without explicit function types:

def foo():
    let p = person{}
    return fn():
        print("hello world")
        p.finished = true
let f = foo()
f()

This generates the following error: error: free variable "p" not in scope

I had a crack at adding a similar error for the explicit function type case here: 11dca27

This, when run on your example code, prevents the crash and instead errors with:
error: cannot pass function value with free variable "p" to declared function type "void_fn"

Sadly, I can't apply this solution at the moment, because while it fixes your case, it now produces this error for a lot of legitimate code, where we use explicit function types, but in a non-escaping manner.

So to properly fix this, I'd first need to add a minimal escape analysis on variables, to filter out those legitimate cases.

So until I am able to get to that (which is a slightly larger task), you're just going to have to be careful not to create function values that have free variables referring to things that will be out of scope by the time it is called :(

@distantforest1
Copy link
Author

distantforest1 commented Jan 22, 2025

I see, it makes sense. If I was possibly adamant on having the function fields, I could have a global person variable or pass it in directly. So it would be accessible from within the context of the function.

def foo():
    return fn(p):
        print("hello world")
        p.finished = true
let f = foo()
f(person{})

or

var p: person? = nil
def foo():
    return fn():
        assert p
        print("hello world")
        p.finished = true
let f = foo()
p = person{}
f()

Making sure person is always visible in the context.

That said though the solution I forwent the use of function fields and changed the talk to a simple bool and ran the outside function when this bool was true. In my case, this simplified version worked.

From the sounds of things I should be wary of using function fields in general? Since the context can vary wildly depending on where it is run?

@aardappel
Copy link
Owner

Yes, passing the person as an argument to the function value is the simplest solution. Or instead of a function value, use a subclass of a class that has talking as a method, and has a person instance variable.

Yes, for the moment you need to be careful with storing function values, as guaranteeing free variables are available is up to you. In most Lobster code, function values are never stored, they are passed as arguments, where they are always statically checked if they use a generic type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants