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

Test return as expression #15548

Merged
merged 1 commit into from
Mar 22, 2016
Merged

Conversation

eschnett
Copy link
Contributor

Closes #11169.

@tkelman
Copy link
Contributor

tkelman commented Mar 21, 2016

needs end-of-file rebase but lgtm


# issue #11169
@test_throws UndefVarError undefvar
let undefvar = return end
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit sketchy --- it's not clear we should even allow return without any obvious enclosing function. Also since this is a let, it wouldn't lead to undefvar being defined globally anyway. Maybe we should test f(return) and confirm that f wasn't called (via a side effect)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should succeed:

uncalled(x) = @test false
fret() = uncalled(return true; @test false)
@test fret()

Alas it doesn't; the @test false after the return true is executed. This looks like a bug to me.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think you're right. I believe this is #9535.

@eschnett
Copy link
Contributor Author

@JeffBezanson I simplified the test -- nothing about argument order any more. LGTY?

JeffBezanson added a commit that referenced this pull request Mar 22, 2016
@JeffBezanson JeffBezanson merged commit 013e0fe into JuliaLang:master Mar 22, 2016
@eschnett eschnett deleted the eschnett/return branch March 22, 2016 15:18
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.

3 participants