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

Machine: Fix arity mismatch with effect handlers due to evidence #163

Merged
merged 3 commits into from
Sep 30, 2022

Conversation

marzipankaiser
Copy link
Contributor

@marzipankaiser marzipankaiser commented Sep 30, 2022

For this code (https://github.com/effekt-lang/effekt/blob/master/examples/llvm/capabilities.effekt):

effect Emit(n: Int) : Unit


def loop(n: Int): Unit / Emit = {
  if (n < 0) {
    ()
  } else {
    do Emit(n);
    loop(n - 1)
  }
}

def main() = {
  try {
    loop(10)
  } with Emit { (n: Int) =>
    println(n); resume(())
  }
}

the following lifted core is generated:

module capabilities

import effekt

record Emit(Emit)

def loop(ev581, ev582, n, Emit$capability) =
  if (infixLt(n, 0)) {
    ()
  } else {
    Emit$capability.Emit(<ev582>, n);
    loop(<ev581>, <ev582>, infixSub(n, 1), Emit$capability)
  }

def main(ev583) =
  try { (ev584, Emit$capability) => 
    loop(<ev584, ev583>, <>, 10, Emit$capability)
  } with Emit {
    def Emit(n, resume) = 
      let _ = println(n);
      resume(<>, ())
  }

()

This, with the current machine transformation, leads to an arity mismatch in both the resume and the call of the effect operation.
A hotfix for this part of the issue is to:

  • Always ignore the first parameter to resume (which is evidence).
  • Ignore the first parameter passed into a handler (which is "its" evidence).

This PR implements this Hotfix and un-ignores the tests fixed by this.

@marzipankaiser
Copy link
Contributor Author

@b-studios Is there always exactly one leading evidence parameter for calls to effect operations and resume?

@marzipankaiser marzipankaiser marked this pull request as ready for review September 30, 2022 14:44
@b-studios
Copy link
Collaborator

At the moment, yes. :)

transform(args).run { values =>
PushStack(Variable(transform(id), Type.Stack()),
Return(values))
Return(values.tail))
Copy link
Collaborator

@b-studios b-studios Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the following would be a bit more explicit about what you are doing:

val evidence :: arguments = values;
PushStack(Variable(transform(id), Type.Stack()),
  Return(arguments))

Copy link
Collaborator

@b-studios b-studios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, one one small nitpick.

@b-studios b-studios merged commit 46952f1 into master Sep 30, 2022
@b-studios b-studios deleted the feature/machine-evidence branch September 30, 2022 16:24
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

Successfully merging this pull request may close these issues.

None yet

2 participants