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

Minor fix for long form anonymous functions #33961

Closed
wants to merge 1 commit into from
Closed

Minor fix for long form anonymous functions #33961

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 27, 2019

Minor fix for #32557. Return type declarations for the long form anonymous functions now return ERROR.

julia> f = function(x)::Int
                  x/2
              end
ERROR: syntax: invalid "::" syntax
Stacktrace:
 [1] top-level scope at REPL[1]:1

To support for the return type declarations, we might have to change the definition of "function" in AST.

@JeffBezanson JeffBezanson added minor change Marginal behavior change acceptable for a minor release triage This should be discussed on a triage call labels Nov 27, 2019
@JeffBezanson
Copy link
Member

To minimize breakage, I think the right thing here might be to give a clear warning like "this use of :: declares the type of the argument, but in the future may change to declare a return type instead".

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Nov 27, 2019
@ararslan ararslan added needs news A NEWS entry is required for this change needs tests Unit tests are required for this change labels Nov 27, 2019
@JeffBezanson
Copy link
Member

Triage thinks we should run PkgEval on this and see if anybody is using this syntax before making a decision.

@JeffBezanson JeffBezanson added needs pkgeval Tests for all registered packages should be run with this change and removed triage This should be discussed on a triage call labels Dec 9, 2019
@JeffBezanson
Copy link
Member

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run tests against primary commit: UndefVarError: LibGit2 not defined

Logs and partial data can be found here
cc @maleadt

@maleadt
Copy link
Member

maleadt commented Dec 16, 2019

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

ProcessExitedException(2)

cc @maleadt

@maleadt
Copy link
Member

maleadt commented Dec 17, 2019

Server crashed.
@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your test job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@JeffBezanson
Copy link
Member

Nice! One thing we find out from PkgEval is that the anon flag used here (which, note, was not used at all in parse-def before) is not entirely accurate. It triggers an error on this case:

function (+)(p::CartesianPoint{T}, v::CartesianVector{T})::CartesianPoint{T} where {T <: Real}

where this initially looks like it might be an anonymous function since the signature begins with (. So this will need a bit more work.

@rfourquet
Copy link
Member

Is something blocking this PR?

@JeffBezanson
Copy link
Member

Yes, see my previous comment. This causes errors on previously valid signatures.

@DilumAluthge
Copy link
Member

It looks like this pull request is no longer mergeable.

If you would still like to make this change, please make a new pull request based on the latest master branch of Julia.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor change Marginal behavior change acceptable for a minor release needs news A NEWS entry is required for this change needs pkgeval Tests for all registered packages should be run with this change needs tests Unit tests are required for this change parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants