Julia: Single argument `isa` function

Created on 13 May 2019  ·  15Comments  ·  Source: JuliaLang/julia

It would be nice to have a single argument isa for cleaner filtering code.

filter(x -> !isa(x, AbstractFloat), [1, 2.0])
# could become
filter(!isa(AbstractFloat), [1, 2.0])

This would provide some consistency with other functions, such as isequal, but since isa is a builtin function, defining other variants doesn't seem trivial.

julia> Core.isa(::Type{T}) where {T} = (x) -> isa(x, T)
ERROR: cannot add methods to a builtin function
help wanted

Most helpful comment

Can I Work on this issue?

All 15 comments

Yes, this matches a bunch of our other single-argument versions of binary operators.

Can I Work on this issue?

Sure, go for it! You'll want to make a PR that:

  1. adds this method
  2. adds it to the appropriate doc string
  3. adds a NEWS entry for it
  4. adds a test for it

Awesome. Do you have any documentation that i can go through to understand the codebase?

There's a section on Developer Docs in the manual: https://docs.julialang.org/en/v1/devdocs/init/. I realized that isa is not a generic function which makes this a fairly complex change. Probably not a good first issue after all.

Given the comments in https://github.com/JuliaLang/julia/pull/37240, it's somewhat clear that a generic function isn't acceptable. This leads me to another thought about how to approach this (as well as https://github.com/JuliaLang/julia/issues/36163): do the currying during lowering.

Here's the idea, apologies if I'm saying something dumb as I mostly only understand IR from IRTools.jl, not the real deal.

The idea here is that I explicitly do not want a dispatch that depends on types, I just want Core.isa(T) to always be transformed into Fix2(isa, T). Looking at

julia> f(T) = isa(T)
f (generic function with 1 method)

julia> @code_lowered f(Int)
CodeInfo(
1 ─ %1 = (isa)(T)
└──      return %1
)

julia> @code_typed f(Int)
CodeInfo(
1 ─     (isa)(T)::Union{}
└──     $(Expr(:unreachable))::Union{}
) => Union{}

I think that if during the name resolution (i.e. once we are sure that isa here refers to Core.isa) if we replaced all instances of statements like

%n = isa(T) 

with

%n = Fix2(isa, T)

then we'd get a curried isa that nobody can add methods to and no existing valid code should be affected.

Is this a valid line of attack? I've discussed this with @tkf, but neither of us know enough about lowering to do this ourselves.

I think this should do the trick:

diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index 4e75874e8a..a079874321 100644
--- a/src/julia-syntax.scm
+++ b/src/julia-syntax.scm
@@ -2201,6 +2201,9 @@
                  ((and (eq? (identifier-name f) '^) (length= e 4) (integer? (cadddr e)))
                   (expand-forms
                    `(call (top literal_pow) ,f ,(caddr e) (call (call (core apply_type) (top Val) ,(cadddr e))))))
+                 ((and (eq? f 'isa) (length= e 3))
+                  (expand-forms
+                    `(call (top Fix2) ,f ,(caddr e))))
                  (else
                   (map expand-forms e))))
          (map expand-forms e)))

If people agree this is the way to go, I can make a PR.

Isn’t that a syntax level replacement? I think this might be better done during lowering, not parsing but I’m not sure.

This is during lowering, but note that lowering doesn't resolve isa:

julia> dump(Meta.@lower x isa T)
Expr
  head: Symbol thunk
  args: Array{Any}((1,))
    1: Core.CodeInfo
      code: Array{Any}((2,))
        1: Expr
          head: Symbol call
          args: Array{Any}((3,))
            1: Symbol isa
            2: Symbol x
            3: Symbol T
        2: Expr
          head: Symbol return
          args: Array{Any}((1,))
            1: Core.SSAValue
              id: Int64 1
      codelocs: Array{Int32}((2,)) Int32[1, 1]
      ssavaluetypes: Int64 2
      ssaflags: Array{UInt8}((0,)) UInt8[]
      method_for_inference_limit_heuristics: Nothing nothing
      linetable: Array{Any}((1,))
        1: Core.LineInfoNode
          method: Symbol top-level scope
          file: Symbol none
          line: Int64 0
          inlined_at: Int64 0
      slotnames: Array{Symbol}((0,))
      slotflags: Array{UInt8}((0,)) UInt8[]
      slottypes: Nothing nothing
      rettype: Any
      parent: Nothing nothing
      edges: Nothing nothing
      min_world: UInt64 0x0000000000000001
      max_world: UInt64 0xffffffffffffffff
      inferred: Bool false
      inlineable: Bool false
      propagate_inbounds: Bool false
      pure: Bool false

Ah, right. I think what I should have said is maybe 'name resolution' rather than lowering?

I think the problem is that we don't want someone to have a local variable or function named isa which is not Core.isa and have the pass effect that.

This cannot possibly be reliably done this way if you still want to keep isa a function.

You are just going to get inconsistent result if the user write f = isa; f(T).

Right, that's why I said

during the name resolution (i.e. once we determine that isa refers to Core.isa)

No that's not enough. You don't have enough information until evaluation time regarding how many arguments there are so you can't do this without having the equivalent of adding a method to isa. The best you could do would be (equivalent to) special case Base.isa in inference to assume no one overload the two argument variant.

No that's not enough. You don't have enough information until evaluation time regarding how many arguments there are so you can't do this without having the equivalent of adding a method to isa.

Ah, of course. I didn't think about the possibility of things like splatting. Thank you for explaining this.

Not even splatting. The use of the object may not be directly tracable to when it is called. e.g.

r = Ref{Any}(isa); #= 5 days later and a hundred global variable assignments apart =#; r[](....)

Was this page helpful?
0 / 5 - 0 ratings