allow standalone dotted operators

[simeonschaub]

This lowers dotted operators to Base.BroadcastOp(op), which allows for passing them as closures to functions. One example where this can be really helpful is for array-of-array type data structures, where you want to map a broadcasted function, so you can just do map(.+, [[1,2], [3,4]], [5, 6]). This came up in #35150, wherehadamard was proposed, because mapping broadcasted multiplication is currently quite cumbersome to write. Does this implementation make sense?

close #34156

[stevengj]

Does this still fuse nested dot operations with other dot calls?

[simeonschaub]

No, this doesn't do any fusing with other dot calls, but I am not sure we really want that, since this would make broadcast fusing less transparent and make it not just a syntactic guarantee anymore. Since it just ends up calling Base.materialize(Base.Broadcasted(op, x...)), it will fuse with Base.Broadcasted objects though, if you explicitely construct them.

[stevengj]

No, I mean if you do e.g. 2 .* sqrt.(x) will it still fuse? I haven't checked whether your lowering operation happens before fusion (bad) or after fusion (good).

[simeonschaub]

Yes, those cases should not be affected at all, since broadcast fusion happens before this expansion. That's also why (.*).(a, b) works just like expected, for example.

[stevengj]

Also needs a NEWS item under "language changes".

My understanding is that the only cases that this affects were previously syntax errors, so that this is a backwards-compatible change?

[stevengj]

@nanosoldier runbenchmarks("broadcast", vs = ":master")

[simeonschaub]

Technically, this change could be breaking, if a package assigned to var".+", for example. I can't imagine that any package would have this do anything other than what's proposed in this PR though.
I will add NEWS and docstring, I just wanted some confirmation, that this is the right way, first.

[simeonschaub]

It should also be fairly easy to make this backwards-compatible in Compat.jl, by just defining var".op" = BroadcastOp(op) for every operator.

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[simeonschaub]

One thing that's still a bit unfortunate is that just typing .* into the REPL still throws a UndefVarError and doesn't get this lowering pass. I looked into this a bit and it looks like this is because plain symbols get special-cased in jl_toplevel_eval_flex. Since this is going to be passed to functions directly most of the time anyways, I don't think that should really be a problem, but it would be nice to get this fixed eventually. Maybe somone who knows that part of the code base a bit better than I do has an idea for how to fix this?

[stevengj]

Seems like it should be changed in the if (jl_is_symbol(e)) branch of interpreter.c:eval_value, which I think is what is eventually executed by jl_toplevel_eval_flex.

[simeonschaub]

I believe this should now work. Please don't ask me how! 😆 It would be good if you or someone else could give this a more thorough review, since I was literally just poking at stuff until it worked.

[JeffBezanson]

I'm in favor of this, but I find it pretty unsettling for a symbol to lower to a non-trivial expression. A small example is something like the @which macro, which assumes a symbol can just be looked up in the calling module. I think there'd be many other similar gotchas in the future.

In hindsight, we should have parsed .+ as (dot +). But, I wonder if we can still do that when .+ is in a context where it isn't called, e.g. just an atom by itself. My thinking is that any macros that look for it must currently only be handling it as the first argument to :call.

[simeonschaub]

But, I wonder if we can still do that when .+ is in a context where it isn't called, e.g. just an atom by itself. My thinking is that any macros that look for it must currently only be handling it as the first argument to :call.

I am afraid, I can't quite follow. Should this whole transformation occur in the parser first? What I like about doing this during lowering is that it's (almost) entirely non breaking. But I agree that parsing as (dot op) would have been the nicer solution.

[JeffBezanson]

If we change the parsing, the main thing that can break is macros --- a macro looking for Symbol(".+") would not find it if it were parsed as Expr(:dot, :+) instead. But, that symbol is currently only useful when it's called, as in Expr(:call, :.+, ...), so I'm guessing any such macros would only handle expressions of that form. Outside that context, it might be safe to change the parsing.

Or, we could potentially keep compatibility by converting (dot +) to .+ in all macro arguments, and converting back on the way out.

[c42f]

Or, we could potentially keep compatibility by converting (dot +) to .+ in all macro arguments, and converting back on the way out.

I do like this idea, but ASTs can come from sources other than macro arguments so I'm not sure it would work well enough for compatibility. Kind of hard to say without testing.

[mbauman]

Might as well pass along kwargs here, too, yeah?

[simeonschaub]

Makes sense! Didn't think of that.

[JeffBezanson]

At some point I'll try implementing my comment above --- keep parsing the same for called operators, parse as (dot +) outside that. Then we can at least PkgEval and see if it breaks anything.

[simeonschaub]

That would be great! I tried implementing your suggestion myself, but didn't get very far with it.

[JeffBezanson]

Replaced by #37583 I assume?

[simeonschaub]

Oh, sure! Just forgot to close this.