mbaz (May 12 2023 at 21:42): mbaz (May 12 2023 at 21:42):I'm starting to use JET to test one of my packages (Gaston), and it's already found a few places with subtly incorrect code :heart:
However, in other cases I'm struggling with false positives. For example, in
if x in ('a', 'b', 'c')
it complains that I may be using missing in a Boolean context (since in can return missing). However, I'm sure that x is never missing. How can I tell JET that? I tried
x::Char
if x in ('a', 'b', 'c')
but it doesn't work. On the other hand, these work:
if x::Char in ('a', 'b', 'c')
if in(x::Char, ('a', 'b', 'c'))
but the code becomes less readable IMO. What's the best way to proceed here?
mbaz (May 12 2023 at 22:07):One more; JET complains that IJulia is not defined:
isdefined(Main, :IJulia) && Main.IJulia.inited
It doesn't seem to take into account that && shortcircuits.
jar (May 12 2023 at 22:47):Can you put the x::Char at the place where x is defined?
mbaz (May 12 2023 at 22:52):Yep! That works. x is actually the iteration variable in an outer for loop, but it turns out I can type assert it there. Thanks!
mbaz (May 12 2023 at 22:56):This one has me completely stumped... I have pl::Vector{Pair} and count::Int:
for z in pl[count+1:end]
makes JET complain with:
┌ @ /home/miguel/rcs/jdev/Gaston/src/gaston_aux.jl:465 pl[count Gaston.:+ 1 Gaston.:(:) lastindex(pl)]
│┌ @ array.jl:925 copyto!(X, firstindex(X), A, first(I), lI)
││┌ @ array.jl:319 Base._copyto_impl!(dest, doffs, src, soffs, n)
│││┌ @ array.jl:327 unsafe_copyto!(dest, doffs, src, soffs, n)
││││┌ @ array.jl:281 srcp = pointer(src, soffs)
│││││┌ @ abstractarray.jl:1243 Base._memory_offset(x, i)
││││││┌ @ abstractarray.jl:1247 Base.elsize(x)
│││││││┌ @ abstractarray.jl:251 Base.elsize(Base.typeof(A))
││││││││┌ @ array.jl:213 Base.aligned_sizeof(T)
│││││││││┌ @ reflection.jl:392 al = Base.datatype_alignment(T)
││││││││││ no matching method found `datatype_alignment(::Type{Pair})`: al = Base.datatype_alignment(T::Type{Pair})
Mind, this code works perfectly fine under normal usage.
mbaz (May 12 2023 at 23:05):Ha, figured it out! This was inside a function f(pl::Vector{Pair}) but I had to declare it as f(pl::Vector{P}) where P <: Pair. Darn transitive (or whatever) types get me all the time :sweat_smile:
mbaz (May 12 2023 at 23:05):The undefined symbol is the one I don't know how to fix yet
Sukera (May 13 2023 at 06:11):JET.jl can't really see through the isdefined check - it doesn't know what exactly you're passing as the Symbol (it just sees the symbol), so that could be improved. Maybe open an issue at JET? Not sure where the improvement would need to be, either there or in the compiler to eliminate the UndefVarError case there..
mbaz (May 13 2023 at 14:33):Yeah, I can see that it's quite difficult to determine statically that during execution no undefined symbol will be found.
Sukera (May 13 2023 at 14:36):I think cases like yours where you explicitly want to check whether IJulia is loaded should be handled by package extensions with 1.9+
mbaz (May 13 2023 at 14:42):I want to keep compatibility with LTS, but I'll look into package extensions. I'm not sure it will do the trick, though -- IJulia is not a dependency, and I need to know if it's both loaded and active.
Brian Chen (May 13 2023 at 15:00):Does JET not provide a way to ignore a block of code?
mbaz (May 13 2023 at 15:03):I just started using it yesterday... I haven't found a way to do that.
mbaz (May 13 2023 at 15:03):But I just skimmed through the docs.
Sukera (May 13 2023 at 15:13):Not that I know of, no
Brian Chen (May 13 2023 at 15:16):Seems like a major omission then. No linter is perfect. Granted the model JET uses may preclude any easy ways of marking this
jar (May 13 2023 at 18:35):If the JET user has higher standards than the code author, the user will want JET errors for code the author doesn't want them for. The user should get the final say on what JET reports.
mbaz (May 13 2023 at 18:50):https://github.com/aviatesk/JET.jl/issues/516
mbaz (May 13 2023 at 18:51):But the user can always run their own tests on the code, right?
jar (May 13 2023 at 18:53):The user wants to use JET to evaluate the code.
mbaz (May 13 2023 at 18:54):And I (as package author) want my tests to pass... my point is that the user can always provide their own JET setup to evaluate code to their standards, regardless of what my own tests do
jar (May 13 2023 at 18:56):Yep
Brian Chen (May 13 2023 at 19:22):JET analyses are also a moving target. Getting warnings where there weren't any before after just a version bump is bad UX for a linter, and most of them take great pains to avoid that. Otherwise I'm not sure people would bother using such tools on CI.
Brian Chen (May 13 2023 at 19:24):Granted, that's a different issue than being able to disable linter diagnostics for a certain section of code.
mbaz (May 13 2023 at 19:47):This is my first time using JET, I'm waiting to see how it goes... I mostly plan to run it before tagging a new version or after significant code changes (same for AQUA)
mbaz (May 13 2023 at 21:33):Argh... I found another kind of false positive: checking if a method exists with hasmethod before calling it, JET complains if the method doesn't exist.
It seems like static-checking a dynamic language like Julia is going to be difficult.
jar (May 13 2023 at 21:42):The hasmethod pattern isn't really any good anyway because it fails if there's internal dispatch.
jar (May 13 2023 at 21:51):It's more flexible to just try the thing and have it return a success or failure value you can handle
mbaz (May 14 2023 at 00:54):Would it better to use applicable?
jar (May 14 2023 at 01:27):No those are equivalent
Mason Protter (May 14 2023 at 01:35):In version 1.10, the compiler will know about hasmethod. Here's version 1.9:
julia> f(x::Int) = x + 1;
julia> g(x) = hasmethod(g, Tuple{typeof(x)});
julia> @code_typed g(1)
CodeInfo(
1 ─ %1 = $(Expr(:foreigncall, :(:jl_get_world_counter), UInt64, svec(), 0, :(:ccall)))::UInt64
│ invoke Base.to_tuple_type(Tuple{Int64}::Any)::Type{Tuple{Int64}}
│ %3 = $(Expr(:foreigncall, :(:jl_gf_invoke_lookup), Any, svec(Any, Any, UInt64), 0, :(:ccall), Tuple{typeof(g), Int64}, :(Base.nothing), :(%1), :(%1)))::Any
│ %4 = (%3 === Base.nothing)::Bool
│ %5 = Core.Intrinsics.not_int(%4)::Bool
└── return %5
) => Bool
julia> @btime g(1)
111.074 ns (2 allocations: 112 bytes)
true
and here is v1.10
julia> f(x::Int) = x + 1;
julia> g(x) = hasmethod(g, Tuple{typeof(x)});
julia> @code_typed g(1)
CodeInfo(
1 ─ return true
) => Bool
julia> @btime g(1)
1.080 ns (0 allocations: 0 bytes)
true
Here's an example
https://github.com/JuliaPreludes/Try.jl#side-notes-on-hasmethod-and-applicable-and-invoke
Mason Protter (May 14 2023 at 01:37):On earlier versions of julia you can use Tricks.jl for this though
julia> using Tricks: static_hasmethod
julia> f(x::Int) = x + 1;
julia> g(x) = static_hasmethod(g, Tuple{typeof(x)});
julia> @code_typed g(1)
CodeInfo(
1 ─ return true
) => Bool
julia> @btime g(1)
1.080 ns (0 allocations: 0 bytes)
true
Be warned, Jameson says this implementation is incorrect, but so far as I'm aware, he still hasn't actually shown us an example where it is incorrect or even explained in what way
mbaz (May 14 2023 at 01:39):why is this stuff not documented?
Mason Protter (May 14 2023 at 01:40):Why is what not documented?
mbaz (May 14 2023 at 01:40):that hasmethod and applicable are not reliable
Mason Protter (May 14 2023 at 01:41):They are reliable.
mbaz (May 14 2023 at 01:43):If I understand @jar correctly, they can return incorrect results in some cases
Mason Protter (May 14 2023 at 01:43):They had unreliable performance mostly because the devs don't want you using them to build interfaces, but then people made Tricks.jl so eventually Jameson relented and made them fast
mbaz (May 14 2023 at 01:43):@Mason Protter In the examples you showed, what is the role of f?
jar (May 14 2023 at 01:44):Well it depends which question you mean to ask hasmethod: if your question is the more limited "does this function have this method" yes it's reliable. If it's "will this call run without a method error", it's not reliable.
Mason Protter (May 14 2023 at 01:44):mbaz said:
If I understand jar correctly, they can return incorrect results in some cases
No, Jar is just pointing out that just because a function has an applicable method, doesn't mean you won't hit an error further down the call stack
Mason Protter (May 14 2023 at 01:45):mbaz said:
Mason Protter In the examples you showed, what is the role of
f?
Some function that only has one method
Mason Protter (May 14 2023 at 01:45):E.g. it gives false on Float64:
julia> @code_typed g(1.0)
CodeInfo(
1 ─ return true
) => Bool
julia> @btime g(1.0)
1.080 ns (0 allocations: 0 bytes)
true
jar said:
Well it depends which question you mean to ask
hasmethod: if your question is the more limited "does this function have this method" yes it's reliable. If it's "will this call run without a method error", it's not reliable.
To put it another way, hasmethod will give you the right answer, but you might still be asking a bad question.
jar (May 14 2023 at 01:47):Yeah. I'd say the question that hasmethod can answer just isn't that useful a question.
mbaz (May 14 2023 at 01:48):What about this case?
(v1.9) julia> f(x::Real) = x + 1
f (generic function with 1 method)
(v1.9) julia> hasmethod(f, Tuple{Float64})
true
but f has no method for tuples, right?
Mason Protter (May 14 2023 at 01:49):hasmethod(f, Tuple{Float64}) is asking if f has a method for f(::Float64), not f(::Tuple{Float64}).
Mason Protter (May 14 2023 at 01:50):I.e. hasmethod(f, Tuple{T, U, V}) is asking if f(::T, ::U, ::V) exists.
mbaz (May 14 2023 at 01:50):Oh, darn, I forgot about hasmethod taking a tuple of types :face_palm:
Mason Protter (May 14 2023 at 01:50):jar said:
Yeah. I'd say the question that
hasmethodcan answer just isn't that useful a question.
In general no, but for interfaces, yeah I'd say it can be useful.
mbaz (May 14 2023 at 01:50):OK, I think I get it now.
Mason Protter (May 14 2023 at 01:51):i.e. a classic one is hasmethod(iterate, Tuple{T}) to find out if you have an iterable.
mbaz (May 14 2023 at 01:52):In my case, I just want to make sure a method exists for a certain combination of argument types.
jar (May 14 2023 at 01:54):One way to find that out is to call the function with those arguments and see what happens
mbaz (May 14 2023 at 01:54):With try/catch, right?
Mason Protter (May 14 2023 at 01:54):yuck
mbaz (May 14 2023 at 01:55):I don't love it
jar (May 14 2023 at 01:56):What's the situation where you want to check whether a method exists?
mbaz (May 14 2023 at 01:58):This is for my gnuplot frontend, Gaston. To simplify, let's say you can plot with plot(x, y). Gnuplot only understands real data in a certain format, so I need to convert whatever x and y are to this format.
mbaz (May 14 2023 at 01:59):Gaston handles the usual data types, but I want to let users define their own conversions.
mbaz (May 14 2023 at 02:00):So I provide a function convert_args for users to extend. So if users call plot(x::MyType, y::MyType), it works as long as users also provide convert_args(x::MyType, y::MyType) which returns data in gnuplot-compatible format
mbaz (May 14 2023 at 02:01):Before calling convert_args, I check (with hasmethod) if such a method exists. This has been working fine for a long time, but JET flagged it.
jar (May 14 2023 at 02:02):Why do you check instead of just letting it fail?
Mason Protter (May 14 2023 at 02:03):Yeah, that seems like a case where I'd just let it fail
mbaz (May 14 2023 at 02:03):Good question -- I guess the idea is to provide a nice error message explaining what the problem is and how to solve it.
Mason Protter (May 14 2023 at 02:04):If you want to give them a good error message, you could always just do
convert_args(x, y) = error(ArgumentError("your types haven't yet implemented `convert_args` ..."))
That looks indeed like a better solution, I think I'll go with it.
Mason Protter (May 14 2023 at 02:05):Or even better, use Base.Experimental.register_error_hint.
Mason Protter (May 14 2023 at 02:06):https://docs.julialang.org/en/v1/base/base/#Base.Experimental.register_error_hint
mbaz (May 14 2023 at 02:06):Yeah, I was just looking it up
mbaz (May 14 2023 at 02:07):Looks like it's worth a try
mbaz (May 14 2023 at 02:11):@Mason Protter Coming back to the examples you posted above, g is defined as g(x) = hasmethod(g, Tuple{typeof(x)}), but shouldn't it be g(x) = hasmethod(f, Tuple{typeof(x)})?
Mason Protter (May 14 2023 at 02:12):ha ha ha, yeah it should
Mason Protter (May 14 2023 at 02:12):I should read what I write more carefully
mbaz (May 14 2023 at 02:13):No problem :big_smile: I just wanted to make sure I understood what was going on
mbaz (May 14 2023 at 02:14):BTW thanks a lot for your help @Mason Protter @jar
Sukera (May 15 2023 at 06:32):arguably throw(ArgumentError(...)) is cleaner than error(ArgumentError(...)) :eyes:
Last updated: Nov 22 2024 at 04:41 UTC