Add support for . lifting

[davidanthoff]

Implements #8.

@TotalVerb Would you mind taking a quick look at this? This was mindless copy-paste until things worked :)

I still need to:

• Write tests
• Remove all the white list lifting we no longer need due to this

[TotalVerb]

Generally this looks correct, except possibly a missing promote_containertype method or two. It's a little annoying that promote_containertype suffers from a problem of having to define O(n^2) methods for n distinct containertypes... that's something we should consider fixing in Base.

[TotalVerb]

Does this play well with Nullable? What should broadcasting over DataValue and Nullable do? I think currently it might throw an error?

[davidanthoff]

I'v added two more promote_containertype methods that handle the mixed DataValue and Nullable case, and say that it will return DataValue. Did I do this correct? Was there another concern here that you had?

[TotalVerb]

👍 , this looks right now.

[TotalVerb]

Since this shares Base.hasvalue and Base.unsafe_get with Nullable, when mixing DataValue with Nullable the result is null if any argument is null. That does make some degree of sense to me; but if so, what's the difference between DataValue and Nullable if they're interchangeable in this way?

[davidanthoff]

So ideally I'd like mixed cases to return a DataValue in all cases. With my latest push I'm pretty close, the following all work:

julia> DataValue(3) .+ Nullable()
DataValue{Union{}}()

julia> DataValue() .+ Nullable(4)
DataValue{Union{}}()

julia> DataValue() .+ Nullable()
DataValue{Union{}}()

The thing that doesn't work is this:

julia> DataValue(3) .+ Nullable(2)
ERROR: MethodError: no method matching +(::Int64, ::Nullable{Int64})
Closest candidates are:
  +(::Any, ::Any, ::Any, ::Any...) at operators.jl:424
  +(::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}, ::T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8}) where T<:Union{Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8} at int.jl:32
  +(::Real, ::Complex{Bool}) at complex.jl:238
  ...
Stacktrace:
 [1] broadcast_c at C:\Users\anthoff\.julia\v0.6\DataValues\src\broadcast.jl:23 [inlined]
 [2] broadcast(::Function, ::DataValues.DataValue{Int64}, ::Nullable{Int64}) at .\broadcast.jl:434

If you have any suggestion how to make that case also work and return a DataValue, it would be great!

I'm doing all of this because I need a type that behaves like what I proposed in JuliaLang/julia#21376 for Query.jl, and I don't want to wait until this is sorted out in base...

[TotalVerb]

It's because your unsafe_get is local to this module, not Base.unsafe_get. It should work if you import Base: unsafe_get.

[davidanthoff]

Perfect, that did the trick!

[davidanthoff]

@TotalVerb One last question: Should I see the S::Any in the following, or is that a problem?

julia> foo() = DataValue(3) .+ DataValue(5)
foo (generic function with 1 method)

julia> @code_warntype foo()
Variables:
  #self#::#foo
  nonnull::Bool
  S::Any
  #temp#@_4::Bool
  #temp#@_5::DataValues.DataValue{Int64}

Body:
  begin
      $(Expr(:inbounds, false))
      # meta: location broadcast.jl broadcast 434
      SSAValue(2) = $(Expr(:new, DataValues.DataValue{Int64}, true, 5))
      # meta: location C:\Users\anthoff\.julia\v0.6\DataValues\src\broadcast.jl broadcast_c 20
      nonnull::Bool = (Base.and_int)(true, (Base.and_int)((Core.getfield)(SSAValue(2), :hasvalue)::Bool, true)::Bool)::Bool # line 22:
      goto 8
      8:  # line 23:
      #temp#@_5::DataValues.DataValue{Int64} = $(Expr(:new, DataValues.DataValue{Int64}, :(nonnull), :((Base.add_int)(3, (Core.getfield)(SSAValue(2), :value)::Int64)::Int64)))
      goto 13 # line 28:
      13:
      # meta: pop location
      # meta: pop location
      $(Expr(:inbounds, :pop))
      return #temp#@_5::DataValues.DataValue{Int64}
  end::DataValues.DataValue{Int64}

[TotalVerb]

Nope, it's not a problem. S got optimized out, so inference doesn't bother inferring it (note that it doesn't actually occur in the code). See JuliaLang/julia#20801.

[davidanthoff]

@TotalVerb Thanks so much for your help with this, couldn't have done it without that!

[TotalVerb]

No problem, glad you got it working! I opened an issue here about what we can do about the number of methods needed for promote_containertype, which is not so bad here but can only get worse as more types support broadcast.