Skip to content

Conversation

@mlechu
Copy link
Member

@mlechu mlechu commented May 2, 2025

On nightly (going back to at least 1.10):

julia> Base.Experimental.@opaque ((x::T,y::T) where {T}) -> 123
(ssavalue 3)
(lambda ()
  (() () 0 ())
  (block (call (core apply_type) (core Tuple) (ssavalue 3) (ssavalue 3))
	 (call (core UnionAll) (ssavalue 3) (ssavalue 4))
	 (call (core apply_type) (core Union))
	 (new_opaque_closure (ssavalue 5) (ssavalue 6) (core Any) (true)
			     (opaque_closure_method
			      (null) 2 (false)
			      #1=(line 1 REPL[1])
			      (lambda (#self# x y)
				(((#self# #0=(core Any) 0)
				  (x #0# 0)
				  (y #0# 0))
				 () 0 ())
				(block #1# (line #f) (return 123)))))
	 (return (ssavalue 7))))
ERROR: syntax: ssavalue with no def

method-def-expr- binds static parameters to ssavalues, assigning these
ssavalues in the beginning of the method signature, and then references the
ssavalues when constructing the final svec. Example of such a signature:

;; julia> ((x::T,y::T) where {T}) -> 123
(block (= (ssavalue 0) (call (core TypeVar) 'T))
       (call (core svec)
             (call (core svec)
                   (call (core Typeof) |#22|) (ssavalue 0) (ssavalue 0))
             (call (core svec) (ssavalue 0))
             (inert (line 1 |REPL[1]|))))

When expanding opaque closures, we currently take the svec from
method-def-expr- and forget about the assignments (hence "ssavalue with no
def"). This was probably an oversight, because they've already been assigned to
an unused variable :). This change just keeps the assignments block-prepended
to the argtype.

Since we require the argtype to be/return a tuple (this test case wraps the
tuple in a UnionAll), this change really only upgrades the error we get:

julia> Base.Experimental.@opaque ((x::T,y::T) where {T}) -> 123
ERROR: OpaqueClosure argument tuple must be a tuple type

Fix #49659

Co-authored-by: Jameson Nash <[email protected]>
@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing compiler:lowering Syntax lowering (compiler front end, 2nd stage) backport 1.12 Change should be backported to release-1.12 labels May 2, 2025
@IanButterworth IanButterworth merged commit 7b64cec into JuliaLang:master May 3, 2025
10 checks passed
KristofferC pushed a commit that referenced this pull request May 5, 2025
@KristofferC KristofferC mentioned this pull request May 5, 2025
53 tasks
@KristofferC KristofferC added backport 1.12 Change should be backported to release-1.12 and removed backport 1.12 Change should be backported to release-1.12 labels May 5, 2025
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label May 5, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label May 9, 2025
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this pull request May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lowering error with opaque closures

5 participants