effekt-lang / effekt

A language with lexical effect handlers and lightweight effect polymorphism
https://effekt-lang.org
MIT License
334 stars 24 forks source link

Try to improve resolution by adding a bit context #671

Closed b-studios closed 2 weeks ago

b-studios commented 2 weeks ago

This is an attempt to improve on #670. However, there are some trade-offs as can be seen in the modified test.

Another thing is that the error messages are really bad now if something cannot be found.

b-studios commented 2 weeks ago

Currently, there is an issue where in bytearray we have

def resize(source: ByteArray, size: Int): ByteArray = {
  val target = allocate(size)
  val n = min(source.size, target.size)
  def go(i: Int): ByteArray =
    if (i < n) {
      target.unsafeSet(i, source.unsafeGet(i))
      go(i + 1)
    } else {
      target
    }
  go(0)
}

There the parameter size (a value) shadows the selector .size.

I given this situation, we shouldn't allow values (like parameters etc.) to be called with uniform function call syntax. That's requires some more changes to Namer, however.

That is, we shouldn't reuse resolveFunctionCalltarget for both function calls and method calls.

jiribenes commented 2 weeks ago

Would specialising on arity help us further? If there's something like foo.bar, then perhaps it's more likely to be field access than a method/UFCS'd function?

(I'd personally never write capability.method or object.method without parens, I only tend to do that with UFCS functions and field access 🤔)

b-studios commented 2 weeks ago

Hopefully 566f804 fixes things. However, it would be worth looking at the changed tests (and actually changing them back and look at the errors) to decide what we want to do.

b-studios commented 2 weeks ago

In 4982b9c, I was able to improve the precision a bit further and revert some changes to tests.

The strategy is now to first try the respective function / operation lookup and if this fails completely then also try the other... you never know ;)

b-studios commented 2 weeks ago

Maybe the special casing of error messages is useless, but I'll merge this so that we can have this in the next release.