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

Make 'return;' *actually* return #127

Closed
wants to merge 1 commit into from
Closed

Conversation

hoelzro
Copy link
Contributor

@hoelzro hoelzro commented Sep 15, 2013

No description provided.

@jnthn
Copy link
Contributor

jnthn commented Sep 18, 2013

This way ends up with us having different code paths for return with an argument and return without. Furthermore, it means that the places you can use one or the other are different (anywhere a term is valid for the existing prefix, vs. top level statement for the other case).

I think the righter answer is to unify both, as a term:sym that optionally takes an argument (parsed with ? or so).

@pmichaud
Copy link
Contributor

I agree, I prefer a single path and not separate ones.
Also, are there (m)any cases where NQP really needs a bare return?

Pm

On Wed, Sep 18, 2013 at 07:24:35AM -0700, Jonathan Worthington wrote:

This way ends up with us having different code paths for return with an argument and return without. Furthermore, it means that the places you can use one or the other are different (anywhere a term is valid for the existing prefix, vs. top level statement for the other case).

I think the righter answer is to unify both, as a term:sym that optionally takes an argument (parsed with ? or so).


Reply to this email directly or view it on GitHub:
#127 (comment)

@hoelzro
Copy link
Contributor Author

hoelzro commented Sep 18, 2013

I would also prefer a single return path, but I didn't have the know-how to do it. NQP doesn't really need a bare return, but the problem I encountered that motivated me to do this was something like this:

sub my-sub() {
  say('here');
  return;
  say('there');
}

When I tried this over the weekend (for debugging reasons), the return was simply skipped over, leading to much confusion on my part. It took me a while to figure out what was going on, and I wanted to spare future NQP developers that confusion.

@pmichaud
Copy link
Contributor

Then perhaps

NQP 'return' requires an argument at line ...

That can be done in the existing grammar rule without too much
difficulty. :-)

Pm

@jnthn
Copy link
Contributor

jnthn commented Sep 18, 2013

@pmichaud the "return needs an argument" thing does catch people out. Even me. :-) So we should fix it, really. We can't so easily detect it with the exist grammar rule 'cus it parses as a prefix operator. I mean, we could do a lookahead like <!before <.ws> <[;}]>> or so...but that feels a little icky..

@pmichaud
Copy link
Contributor

@jnthn then yes, switching return to be a term with an optional argument can work also, I suppose.

At any rate, I prefer only one code path for parsing 'return', however it ends up.

@arnsholt
Copy link
Contributor

@jnthn Wouldn't something like term:sym<return> { <sym> <EXPR>? } allow a construct like foo(return 1)?

@pmichaud
Copy link
Contributor

@arnsholt Yes, foo(return 1) is valid Perl 6.

@arnsholt
Copy link
Contributor

Oh, in that case it's perfectly okay of course. Are the semantics of foo(return 1) different from those of return 1, or are they identical?

@jnthn
Copy link
Contributor

jnthn commented Sep 18, 2013

Identical. Note that while that's not the most useful example, useful is like $alright || return 'åhnejs'; or so.

@arnsholt arnsholt closed this in 724a9b1 Oct 28, 2013
arnsholt added a commit that referenced this pull request Oct 29, 2013
This way, 'return;' won't be silently ignored in a function. Closes #127.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants