Rework getproperty for LObjects to be inferrable for fields

[timholy]

This addresses one of the points in MakieOrg/Makie.jl#706. If ax is an LAxis, then ax.scene should infer concretely (it refers to a field) whereas ax.ylabel (an attribute) will not.

Demo: here's the function body for LAxis:

julia> L = AbstractPlotting.MakieLayout.LAxis
AbstractPlotting.MakieLayout.LAxis

julia> ex = :(return getfield(layoutable, :attributes)[s])
:(return (getfield(layoutable, :attributes))[s])

julia> for (fn, ft) in zip(fieldnames(L), fieldtypes(L))
               fnq = QuoteNode(fn)
               ex = Expr(:elseif, :(s === $fnq), :(return getfield(layoutable, $fnq)::$ft), ex)
           end

julia> ex.head = :if
:if

julia> ex
:(if s === :decorations
      return getfield(layoutable, :decorations)::Dict{Symbol, Any}
  elseif s === :block_limit_linking
      return getfield(layoutable, :block_limit_linking)::Observable{Bool}
  elseif s === :attributes
      return getfield(layoutable, :attributes)::Attributes
  elseif s === :layoutobservables
      return getfield(layoutable, :layoutobservables)::GridLayoutBase.LayoutObservables
  elseif s === :limits
      return getfield(layoutable, :limits)::Observable{GeometryBasics.HyperRectangle{2, Float32}}
  elseif s === :yaxislinks
      return getfield(layoutable, :yaxislinks)::Vector{AbstractPlotting.MakieLayout.LAxis}
  elseif s === :xaxislinks
      return getfield(layoutable, :xaxislinks)::Vector{AbstractPlotting.MakieLayout.LAxis}
  elseif s === :scene
      return getfield(layoutable, :scene)::Scene
  elseif s === :parent
      return getfield(layoutable, :parent)::Scene
  else
      return (getfield(layoutable, :attributes))[s]
  end)

This is therefore quite similar to https://timholy.github.io/SnoopCompile.jl/stable/snoopr/#Inferrable-field-access-for-abstract-types-1.

[timholy]

Now I'm noticing that there are quite a few other such getproperty methods. I can do the same treatment on most/all of them, but one key question I don't know the answer to: of the abstract types, which might have new subtypes added outside this package? If they are all thought to be "closed" (meaning, all subtypes are generated within this package), then it's easy: I can just eval one such function per type. But if some of them are designed to be extended, we may even want to consider a @generated version that will cover all subtypes.

[jkrumbiegel]

Why does my previous code not infer correctly? I thought that the symbols used in getproperty could constant prop and that because all fieldnames are known at compile time, it should be known what the types are? I really don't like this long list where I have to spell out all the types.. I just saw yesterday that Simon used a different approach for AbstractPlotting attributes, is this better? https://github.com/JuliaPlots/AbstractPlotting.jl/blob/master/src/dictlike.jl#L80-L96

[timholy]

Why does my previous code not infer correctly?

Because fieldnames is not static/pure (it even calls method that is @nospecialized on the type), so the in call doesn't resolve at compile time. So the compiler can't tell whether it will return a field or go to the attributes dictionary, and moreover there's a single callsite for getfield but the different fields have different types.

Here's how you can see the problem for yourself (obviously, don't use this PR, use master or a released version):

julia> using AbstractPlotting, Cthulhu

julia> using AbstractPlotting.MakieLayout

julia> scene, layout = layoutscene();

julia> ax = LAxis(scene);

julia> f(axis) = axis.scene
f (generic function with 1 method)

julia> @descend f(ax)

If you don't know how to use Cthulhu, I can't recommend it highly enough. It's hard to imagine a better use of half an hour. This video is about invalidations but starting at 7:00ish it's basically a Cthulhu tutorial.

[timholy]

In case it's not apparent, the implication is that almost nothing in Makie is inferrable, even the fields that you've gone to the effort to put concrete types on.

[timholy]

Oh, and I missed the second part of your question because I was focused on the first. Yes, https://github.com/JuliaPlots/AbstractPlotting.jl/blob/master/src/dictlike.jl#L80-L96 fixes the problem too---that's basically what I meant above by a @generated version. (Even though I helped create @generated I typically try to avoid it, but this may be a pretty good use case.)

[timholy]

So perhaps you can just delete the specialized getproperty methods since they all seem to be subtypes of Transformable?

[timholy]

I do think if you delete the specialized methods then given https://github.com/JuliaPlots/AbstractPlotting.jl/blob/master/src/dictlike.jl#L80-L96 you're probably OK, so there may be no reason to merge this.

[jkrumbiegel]

thank you, I think I'll look at just applying the existing definitions to my MakieLayout types. I was simply not aware of this mechanism at the time so I duplicated existing structure (and in an inferior way..)