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

Already on GitHub? Sign in to your account

Use CompiledCode name in Proc/BlockEnvironment ArgumentErrors #2169

Merged
merged 3 commits into from Feb 23, 2013

Conversation

Projects
None yet
2 participants
Member

solson commented Feb 19, 2013

When creating ArgumentErrors in Procs and BlockEnvironments it should use the name of the underlying CompiledCode instead of just defaulting to __block__ everywhere.

I first noticed this when working on Apricot (a programming language on Rubinius). When I gave functions the wrong number of arguments like ((fn foo []) 1) I would get a message like:

ArgumentError: method '__block__': given 1, expected 0

When I expected:

ArgumentError: method 'foo': given 1, expected 0

After my patch, this works as expected. I didn't need to change all the places where it defaults to __block__ to achieve this result, but I figured why not while I'm at it? If any of this is a problem, let me know how I can make it better!

Scott Olson added some commits Feb 19, 2013

Scott Olson Check if Proc has a block before using it.
The arity check for lambda mode uses the block... the check for whether there
is a block should come first.
e1673ec
Scott Olson Use CompiledCode's name in Proc argument errors. d82a940
Scott Olson Use CompiledCode's name in BlockEnvironment argument errors. 1079afa
Member

solson commented Feb 19, 2013

The Travis build failure doesn't seem to have anything to do with my patch.

Owner

dbussink commented Feb 19, 2013

The problem here is that we probably should clear on the difference here. The new error message makes it look like there is an error in calling the method, which can be very confusing and point people to the wrong code to fix it.

So yes, I'm all for improving this, but changing it to this style can make it very confusing in certain situations, so we should be very clear on making this situations distinct from calling a method with the wrong number of arguments.

Member

solson commented Feb 19, 2013

I guess I need to be more clear about what my patch does. In Ruby code, I don't think you'd ever notice a difference... calling a lambda with the wrong number of arguments will complain with the same old error message using the __block__ name. But now that's because that's what the lambda's name is (it's been the default name all along) and not because the name is hardcoded in the C++.

Where it will make a difference is when the lambda's name is not __block__ as happens in my language, Apricot, where "anonymous" functions can actually be given names (so they can do recursive calls). And even for the ones that aren't named, I default to the name __fn__ because it makes more sense than __block__ in Apricot.

Does that clear things up or is there something else I missed?

Member

solson commented Feb 19, 2013

In case there's still some confusion, I also want to make it clear that this is all about calling lambdas with the wrong number of arguments. (From your second paragraph I gather this maybe wasn't clear.)

Owner

dbussink commented Feb 23, 2013

Ok, looks like it shouldn't be a problem then :).

@dbussink dbussink added a commit that referenced this pull request Feb 23, 2013

@dbussink dbussink Merge pull request #2169 from tsion/proc-argument-error
Use CompiledCode name in Proc/BlockEnvironment ArgumentErrors
0589507

@dbussink dbussink merged commit 0589507 into rubinius:master Feb 23, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment