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?
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.
Can you put the x::Char
at the place where x
is defined?
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!
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.
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:
The undefined symbol is the one I don't know how to fix yet
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..
Yeah, I can see that it's quite difficult to determine statically that during execution no undefined symbol will be found.
I think cases like yours where you explicitly want to check whether IJulia is loaded should be handled by package extensions with 1.9+
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.
Does JET not provide a way to ignore a block of code?
I just started using it yesterday... I haven't found a way to do that.
But I just skimmed through the docs.
Not that I know of, no
Seems like a major omission then. No linter is perfect. Granted the model JET uses may preclude any easy ways of marking this
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.
https://github.com/aviatesk/JET.jl/issues/516
But the user can always run their own tests on the code, right?
The user wants to use JET to evaluate the code.
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
Yep
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.
Granted, that's a different issue than being able to disable linter diagnostics for a certain section of code.
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)
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.
The hasmethod pattern isn't really any good anyway because it fails if there's internal dispatch.
It's more flexible to just try the thing and have it return a success or failure value you can handle
Would it better to use applicable
?
No those are equivalent
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
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
why is this stuff not documented?
Why is what not documented?
that hasmethod
and applicable
are not reliable
They are reliable.
If I understand @jar correctly, they can return incorrect results in some cases
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
@Mason Protter In the examples you showed, what is the role of f
?
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.
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
mbaz said:
Mason Protter In the examples you showed, what is the role of
f
?
Some function that only has one method
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.
Yeah. I'd say the question that hasmethod
can answer just isn't that useful a question.
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?
hasmethod(f, Tuple{Float64})
is asking if f
has a method for f(::Float64)
, not f(::Tuple{Float64})
.
I.e. hasmethod(f, Tuple{T, U, V})
is asking if f(::T, ::U, ::V)
exists.
Oh, darn, I forgot about hasmethod
taking a tuple of types :face_palm:
jar said:
Yeah. I'd say the question that
hasmethod
can answer just isn't that useful a question.
In general no, but for interfaces, yeah I'd say it can be useful.
OK, I think I get it now.
i.e. a classic one is hasmethod(iterate, Tuple{T})
to find out if you have an iterable.
In my case, I just want to make sure a method exists for a certain combination of argument types.
One way to find that out is to call the function with those arguments and see what happens
With try/catch, right?
yuck
I don't love it
What's the situation where you want to check whether a method exists?
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.
Gaston handles the usual data types, but I want to let users define their own conversions.
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
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.
Why do you check instead of just letting it fail?
Yeah, that seems like a case where I'd just let it fail
Good question -- I guess the idea is to provide a nice error message explaining what the problem is and how to solve it.
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.
Or even better, use Base.Experimental.register_error_hint
.
https://docs.julialang.org/en/v1/base/base/#Base.Experimental.register_error_hint
Yeah, I was just looking it up
Looks like it's worth a try
@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)})
?
ha ha ha, yeah it should
I should read what I write more carefully
No problem :big_smile: I just wanted to make sure I understood what was going on
BTW thanks a lot for your help @Mason Protter @jar
arguably throw(ArgumentError(...))
is cleaner than error(ArgumentError(...))
:eyes:
Last updated: Nov 06 2024 at 04:40 UTC