Dot-broadcasting for short-circuiting ops .&& and .||

[mbauman]

I have long wanted a proper fix for issue #5187. It was the very first Julia issue I filed.
This is a shot at such a fix. This PR:

• Enables parsing for .&& and .||. They are parsed into syntactic Expr(:.&&, ...) expressions at the same precedence as their respective && and ||:

  julia> Meta.show_sexpr(:(a .&& b))
  (:.&&, :a, :b)

• Unlike all other dotted operators .op (like .+), the op-alone part (var"&&") is not an exported name from Base. As such, this effectively lowers to broadcasted((x,y)->x && y, ...), but instead of using an anonymous function I've named it Base.andand and Base.oror:

  julia> Meta.@lower a .&& b
  :($(Expr(:thunk, CodeInfo(
      @ none within `top-level scope'
  1 ─ %1 = Base.broadcasted(Base.andand, a, b)
  │   %2 = Base.materialize(%1)
  └──      return %2
  ))))

• I've used a named function to enable short-circuiting behavior within the broadcast kernel itself. In the case that the second argument is a part of the same fused broadcast kernel, it will only evaluate if required:

  julia> mutable struct F5187; x; end
  
  julia> (f::F5187)(x) = (f.x += x)
  
  julia> (iseven.(1:4) .|| (F5187(0)).(ones(4)))
  4-element Vector{Real}:
      1.0
   true
      2.0
   true

• This also enables support for standalone .&& and .|| as BroadcastFunctions, but of course they are not able to short-circuit when used as functions themselves.

Request for feedback

• A good bikeshed could be had over the names themselves; the implemented andand and oror are cutesy placeholders. We could actually use var"&&" and var"||" for these names — and it'd almost simplify the implementation, but in order to do so we'd have to actually export them from Base, too. It seems like it might just be confusing.
  The names are fine
• Is this the implementation we want? This uses Broadcast.flatten to create the lazy function needed for short-circuiting the second argument. This could alternatively be done directly within the parser — perhaps by resurrecting the old 0.5 broadcast parsing behavior. Someone else would have to do that work if they wanted it.
  Yes
• Do we want to support the stand-alone .&& and .|| BroadcastFunctions if they cannot possibly short circuit?
  Maybe, but deferred
• Should dotted chained comparisons like 1 .< A .< 2 produce (1 .< A) .&& (A .< 2)? It currently uses .&. I'm leaning towards keeping the status quo, even though this would represent a departure from the scalar lowering. I'd bet that changing this would be breaking for folks that are effectively working around Lower chained inequalities in an extendable way #39104 (knowingly or not).
  Maybe, but breaking and deferred

(See also #26792 which made space for this feature.)

[MasonProtter]

Forgive me if I'm misunderstanding, but I think this doesn't correctly preserve the semantics of && / ||. It seems to basically just be & and | instead.

For instance:

julia> @eval Base.Broadcast begin

       andand(a, b) = a && b
       function broadcasted(::typeof(andand), a, bc::Broadcasted)
           bcf = flatten(bc)
           broadcasted((a, args...) -> a && bcf.f(args...), a, bcf.args...)
       end
       oror(a, b) = a || b
       function broadcasted(::typeof(oror), a, bc::Broadcasted)
           bcf = flatten(bc)
           broadcasted((a, args...) -> a || bcf.f(args...), a, bcf.args...)
       end

       end
broadcasted (generic function with 56 methods)

julia> using Base.Broadcast: oror

julia> oror.(iseven.(1:4), (F5187(0)).(ones(4)))
4-element Vector{Real}:
    1.0
 true
    2.0
 true

julia> oror.(isfinite.(1:4), throw("nonfinite number!"))
ERROR: "nonfinite number!"
Stacktrace:
 [1] top-level scope
   @ REPL[8]:1

That is, this appears to not really short circuit since the both the function arguments get evaluated.

---

Edit:

Okay, so it will work if I also broadcast the throw:

julia> oror.(isfinite.(1:4), throw.("nonfinite number!"))
4-element BitVector:
 1
 1
 1
 1

I'm not sure how to feel about this. Is there a way to make the first one work without throw.(...)? I can kinda see the logic of demanding the . here as it basically says "fuse the call to throw with the loop".

So it seems I was just misunderstanding and this was intended design all along.

[simeonschaub]

Overall, this makes sense to me, but I am a bit worried about how this will play together with some custom broadcast implementations. Not really sure how to test that though.

[simeonschaub]

@MasonProtter Non-broadcasted calls are always evaluated before broadcasted ones and I don't really see any way around this. To me at least, this behavior for throw makes sense.

[simeonschaub]

What do you think about using callable objects AndAnd() and OrOr() instead of regular functions, since we might not want e.g. andand isa Function?

[mbauman]

That's also what I did at first... but for their BroadcastFunction forms to work they need to implement (a,b) -> a && b... and if they implement that, well, aren't they functions? That's really the crux of the question — do we want to them to be callable at all? If they're callable, I think they might as well just be functions.

[simeonschaub]

Yeah, perhaps we don't even want to make them callable. It seems to me like anywhere those are called directly is probably likely to be a bug.

[MasonProtter]

Non-broadcasted calls are always evaluated before broadcasted ones and I don't really see any way around this. To me at least, this behavior for throw makes sense.

@simeonschaub Yeah, I've warmed considerably to this, but I will note that the the standard behaviour of executing calls is built around assuming that the objects being broadcasted are functions, and && / || are not functions and have different semantics. Namely, that the thing to the right of them only gets evaluated if the thing to the left is true (false).

I still can't help but feel this is semantically a bit different.

[mbauman]

standard behaviour of executing calls is built around assuming that the objects being broadcasted are functions, and && / || are not functions and have different semantics.

I see this a little differently: I see a fused broadcast as being a simple syntax for a complicated for loop. This is most clearly demonstrated with the @. macro, but the mental model should be that anything that's not dotted is hoisted out of the loop. It doesn't really matter what is inside the for loop. I'd want this @. A < 1 || A > 2 to behave the same as A .< 1 .|| A .>2 to behave (roughly) the same as [A[i] < 1 || A[i] > 2 for i in eachindex(A)]. The dots just put it (whatever it is) inside the loop.

[mbauman]

On the topic of how broadcast implementors would see this: I intentionally intercept this very early on in the call chain. There are two forms of Broadcasteds this uses:

• Broadcasted(andand, (x, y)) — this is where it makes sense for andand to be a Function. There's no evaluation to be done in the second argument and thus no short circuiting — and so this really wouldn't be any different than any other Julia function.
• Broadcasted((x, yargs...)->x && f(yargs...), (x, yargs...)) — this is the case where there's some function evaluation happening in the second argument. As such, we build a flattened function f to represent the second argument and lift out all the arguments it uses, and then we use an anonymous function to put it together. All the custom implementations see is an anonymous function with a bunch of arguments — something they're already handling regularly. An advantage of doing this in Julia is that in building the flattened f we go through all the normal machinations of broadcasting. Interestingly, this means that, e.g., the O(1) specializations (like immediate computation of ranges) will still occur even if the LHS is all false.

This really isn't any different from what they deal with regularly.

[JeffBezanson]

• Do we want to support the stand-alone .&& and .|| BroadcastFunctions if they cannot possibly short circuit?

I would say probably yes just for consistency.

• Should dotted chained comparisons like 1 .< A .< 2 produce (1 .< A) .&& (A .< 2)? It currently uses .&. I'm leaning towards keeping the status quo

From triage: changing it would be the right thing but breaking, so keep it as-is for now.

[mbauman]

Thanks Simeon! I've updated the first post to reflect the current status quo.

This is ready as far as I'm concerned.

[simeonschaub]

I think I would still lean towards oror and andand not being callable, as to not add any confusion with regular functions. I am open to being convinced otherwise though.

[mbauman]

If they're not callable, we still need to implement this method. Here's a potential diff (of course, typeof(andand) could be simplified to AndAnd, but limiting diff size here):

diff --git a/base/broadcast.jl b/base/broadcast.jl
index 60ea22805f..cbd7cc9c86 100644
--- a/base/broadcast.jl
+++ b/base/broadcast.jl
@@ -179,12 +179,16 @@ function Broadcasted{Style}(f::F, args::Args, axes=nothing) where {Style, F, Arg
     Broadcasted{Style, typeof(axes), Core.Typeof(f), Args}(f, args, axes)
 end

-andand(a, b) = a && b
+struct AndAnd end
+const andand = AndAnd()
+broadcasted(::typeof(andand), a, b) = broadcasted((a, b) -> a && b, a, b)
 function broadcasted(::typeof(andand), a, bc::Broadcasted)
     bcf = flatten(bc)
     broadcasted((a, args...) -> a && bcf.f(args...), a, bcf.args...)
 end
-oror(a, b) = a || b
+struct OrOr end
+const oror = OrOr()
+broadcasted(::typeof(oror), a, b) = broadcasted((a, b) -> a || b, a, b)
 function broadcasted(::typeof(oror), a, bc::Broadcasted)
     bcf = flatten(bc)
     broadcasted((a, args...) -> a || bcf.f(args...), a, bcf.args...)

I actually started out this direction, but I changed to this because it's simpler (4 fewer LOC, and it lets Julia handle the singleton for us) and we are effectively defining a function method anyway. And it enabled the BroadcastFunction (which we've now dropped), but it also felt less user-hostile in case it leaked out.

[simeonschaub]

but it also felt less user-hostile in case it leaked out.

My worry is that these leaking out is likely to indicate a bug, since treating them just like regular functions would ignore their short-circuiting behavior and could lead to subtle mistakes.

[simeonschaub]

I went ahead with this change for now, but feel completely free to revert if you'd prefer the previous approach.

[simeonschaub]

@mbauman Apparently, I can't request approval from you for your own PR, but please let me know what you think.

[mbauman]

Yes, I'm ok with this — not my first choice, but also not worth holding up the feature for. :)

Apparently I can't make a green check on my own PR, either:

Here you go: