Remove drop order twist of && and || and make them associative by est31 · Pull Request #103293 · rust-lang/rust (original) (raw)

Previously a short circuiting binop chain (chain of && or ||s) would drop the temporaries created by the first element after all the other elements, and otherwise follow evaluation order. So f(1).g() && f(2).g() && f(3).g() && f(4).g() would drop the temporaries in the order 2,3,4,1. This made && and || non-associative regarding drop order. In other words, adding ()'s to the expression would change drop order: f(1).g() && (f(2).g() && f(3).g()) && f(4).g() for example would drop in the order 3,2,4,1.

As, except for the bool result, there is no data returned by the sub-expressions of the short circuiting binops, we can safely discard of any temporaries created by the sub-expr. Previously, code was already putting the rhs's into terminating scopes, but missed it for the lhs's.

This commit addresses this "twist". We now also put the lhs into a terminating scope. The drop order of the above expressions becomes 1,2,3,4.

There might be code relying on the current order, and therefore I'd recommend doing a crater run to gauge the impact. I'd argue that such code is already quite wonky as it is one foo() && addition away from breaking. For the impact, I don't expect any build failures, as the compiler gets strictly more tolerant: shortening the lifetime of temporaries only expands the list of programs the compiler accepts as valid. There might be runtime failures caused by this change however. Edit: both build and runtime failures are possible, e.g. see the example provided by dtolnay below. Edit2: the crater run has finished and results are that there is only one build failure which is easy to fix with a +/- 1 line diff.

I've included a testcase that now compiles thanks to this patch.

The breakage is also limited to drop order relative to conditionals in the && chain: that is, in code like this:

let hello = foo().hi() && bar().world(); println!("hi");

we already drop the temporaries of foo().hi() before we reach "hi".

I'd ideally have this PR merged before let chains are stabilized. If this PR is taking too long, I'd love to have a more restricted version of this change limited to &&'s in let chains: the &&'s of such chains are quite special anyways as they accept let bindings, in there the && is therefore more a part of the "if let chain" construct than a construct of its own.

Fixes #103107

Status: waiting on this accepted FCP finishing.