Skip to content

Commit

Permalink
fix #44328: method validation for opaque closures (#44335)
Browse files Browse the repository at this point in the history
I believe it's intentional that for these methods, the `sig` field is
just ignored and always set to `Tuple`. Also fixes a lowering bug I
discovered that would cause errors if `Union` was shadowed.

I have verified that this fixes the reported warnings.

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
  • Loading branch information
simeonschaub and vtjnash authored Mar 1, 2022
1 parent 3a47c1c commit 5deb503
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
5 changes: 4 additions & 1 deletion base/compiler/validation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const NON_TOP_LEVEL_METHOD = "encountered `Expr` head `:method` in non-top-level
const NON_TOP_LEVEL_GLOBAL = "encountered `Expr` head `:global` in non-top-level code (i.e. `nargs` > 0)"
const SIGNATURE_NARGS_MISMATCH = "method signature does not match number of method arguments"
const SLOTNAMES_NARGS_MISMATCH = "CodeInfo for method contains fewer slotnames than the number of method arguments"
const INVALID_SIGNATURE_OPAQUE_CLOSURE = "invalid signature of method for opaque closure - `sig` field must always be set to `Tuple`"

struct InvalidCodeError <: Exception
kind::String
Expand Down Expand Up @@ -215,7 +216,9 @@ function validate_code!(errors::Vector{>:InvalidCodeError}, mi::Core.MethodInsta
m = mi.def::Method
mnargs = m.nargs
n_sig_params = length((unwrap_unionall(m.sig)::DataType).parameters)
if (m.isva ? (n_sig_params < (mnargs - 1)) : (n_sig_params != mnargs))
if m.is_for_opaque_closure
m.sig === Tuple || push!(errors, InvalidCodeError(INVALID_SIGNATURE_OPAQUE_CLOSURE, (m.sig, m.isva)))
elseif (m.isva ? (n_sig_params < (mnargs - 1)) : (n_sig_params != mnargs))
push!(errors, InvalidCodeError(SIGNATURE_NARGS_MISMATCH, (m.isva, n_sig_params, mnargs)))
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -3856,7 +3856,7 @@ f(x) = yt(x)
v)))
cvs)))
`(new_opaque_closure
,(cadr e) (call (core apply_type) Union) (core Any)
,(cadr e) (call (core apply_type) (core Union)) (core Any)
(opaque_closure_method (null) ,nargs ,isva ,functionloc ,(convert-lambda lam2 (car (lam:args lam2)) #f '() (symbol-to-idx-map cvs)))
,@var-exprs))))
((method)
Expand Down

0 comments on commit 5deb503

Please sign in to comment.