Single argument isa function

[krrutkow]

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

[StefanKarpinski]

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

[badrimurali]

Can I Work on this issue?

[StefanKarpinski]

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

[badrimurali]

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

[StefanKarpinski]

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.

[MasonProtter]

Given the comments in #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 #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.

[simeonschaub]

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.

[MasonProtter]

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

[simeonschaub]

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

[MasonProtter]

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.

[yuyichao]

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).

[MasonProtter]

Right, that's why I said

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

[yuyichao]

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.

[MasonProtter]

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.

[yuyichao]

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[](....)