methoderror for constructors considers typevars #33613
Conversation
| print(iob, use_constructor_syntax ? func : typeof(func).name.mt.name) | ||
| end | ||
| print(iob, "(") | ||
| print(iob, " ", func, "(") |
There was a problem hiding this comment.
Something like this to still highlight mismatches, like other arguments?
| print(iob, " ", func, "(") | |
| print(iob, " ") | |
| if !isa(func, rewrap_unionall(s1, method.sig)) | |
| ... Base.with_output_color(...); print(iob, func); end ... | |
| else | |
| print(iob, func) | |
| end | |
| print(iob, "(")) |
There was a problem hiding this comment.
IMO that will be a very bad effort/result ratio, it the ... are extended. I would prefer to leave it as is.
| for method in methods(func) | ||
| for (f, arg_types_param) in funcs | ||
| f2 = f isa UnionAll ? unwrap_unionall(unwrap_unionall(f).name.wrapper) : f | ||
| for method in methods(f2) |
There was a problem hiding this comment.
methods(unwrap_unionall(...)) is an invalid query—this must be called only on a properly wrapped object (e.g. the .wrapper)
There was a problem hiding this comment.
That was the original suggestion in #33539 (comment), and it obviously works for DataType.
Using the methods(... .name.wrapper) does not deliver all the methods we are after.
How would I have to "properly wrap" the object?
There was a problem hiding this comment.
Ah, right. methods can't do this query properly (which is its own shortcoming). You'd need to bypass it and call into the Base._methods_by_ftype function directly with f2 isa Type ? Type{<:f2} : typeof(f2).
There was a problem hiding this comment.
I don't see the point: If I @enter methods(f2) I end at
830 tt = signature_type(f, t)
831 return _methods_by_ftype(tt, lim, world)
1|julia> tt
Tuple{Type{Foo{T<:Integer,S,U}},Vararg{Any,N} where N}
That seems to be exactly the same, what you propose to call directly. The results are identical. Only difference is,
the methods throws an ArgumentError, if f2 isa Core.Builtin. So why shouldn't I simply call methods(f2)??
|
Bump. |
|
@vtjnash has been traveling but it would be good to make forward progress on this. |
|
there has been progress with the changed files, which resolve the issue better than this PR. |
fix #33539