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

Check function arity before evaluating arguments #515

Closed
wants to merge 1 commit into from

Conversation

@Hamled
Copy link
Contributor

commented Oct 7, 2019

If the function is not going to be executed due to the thrown RuntimeError it makes sense to also not evaluate the arguments that are part of the call expression.

One thing to note is that the order of the text explaining the original visitCallExpr snippet is no longer exactly matching the order of the lines of code. I couldn't think of a simple way to re-organize the text based on its current form, but it also doesn't seem like it would confuse readers.

If the function is not going to be executed due to the thrown
RuntimeError it makes sense to also not evaluate the arguments that are
part of the call expression.
@munificent

This comment has been minimized.

Copy link
Owner

commented Oct 11, 2019

It makes sense — there's no use in evaluating the arguments first since it's going to die. But I do think users expect the arguments to be evaluated before the arity is checked. Arity checking is part of the function itself and I think most users intuit that the invocation doesn't "begin" at all until after the arguments are evaluated.

I think their mental model is:

  1. Function expression is evaluated. (Usually just loading a variable for the function's name.)
  2. Arguments are evaluated.
  3. Parameters are bound.
  4. Function body is run.

Arity checking is conceptually part of 3. I believe this is how most other dynamic languages work. For example, consider this Ruby program:

def foo(a, b)
  puts "#{a} #{b}"
end

def trace(n)
  puts "eval #{n}"
  n
end

foo(trace(1), trace(2), trace(3))

If you run it, it prints:

eval 1
eval 2
eval 3
ruby.rb:1:in `foo': wrong number of arguments (given 3, expected 2) (ArgumentError)
	from ruby.rb:10:in `<main>'

So it's behaving the way Lox does.

@munificent munificent closed this Oct 11, 2019
@Hamled Hamled deleted the Hamled:arity-check branch Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.